From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49386) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WkUnc-0005Gi-Al for qemu-devel@nongnu.org; Wed, 14 May 2014 04:44:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WkUnV-0003JP-Fj for qemu-devel@nongnu.org; Wed, 14 May 2014 04:44:28 -0400 Received: from mail-ee0-x22c.google.com ([2a00:1450:4013:c00::22c]:63547) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WkUnV-0003JK-4W for qemu-devel@nongnu.org; Wed, 14 May 2014 04:44:21 -0400 Received: by mail-ee0-f44.google.com with SMTP id c41so1103230eek.3 for ; Wed, 14 May 2014 01:44:20 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <53732CDD.5000507@redhat.com> Date: Wed, 14 May 2014 10:44:13 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1399993456-11723-1-git-send-email-pbonzini@redhat.com> In-Reply-To: <1399993456-11723-1-git-send-email-pbonzini@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] KVM: x86: get/set CPL separately from CS List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: gleb@kernel.org, jan.kiszka@siemens.com, avi.kivity@gmail.com, kvm@vger.kernel.org Il 13/05/2014 17:04, Paolo Bonzini ha scritto: > KVM used to assume that CS.RPL could always be used as the CPL > value when KVM_SET_SREGS is called. QEMU could call KVM_GET_SREGS > and the KVM_SET_SREGS exactly after CR0.PE has been set to 1, > but before the long jump that reloads CS. Then, KVM would reset > the CPL to bits 0-1 of CS (aka CS.RPL). > > To avoid this change, KVM introduced a capability that repurposes > a padding byte in struct kvm_segment as the CPL. Detect this > capability and, if it is present, enable it; then get/set the CPL > together with CS. > > The CPL is already migrated separately, as part of hflags. However, > the migrated value might be wrong and we need to fix it up in > the post_load callback. > > Signed-off-by: Paolo Bonzini This patch is also obsolete, using SS.DPL is a simpler solution. > --- > Matching the kernel patches just posted, for now without > a separate patch to update the kernel headers. > > linux-headers/asm-x86/kvm.h | 5 ++++- > linux-headers/linux/kvm.h | 3 +++ > target-i386/kvm.c | 25 ++++++++++++++++++++++--- > target-i386/machine.c | 18 +++++++++++++++++- > 4 files changed, 46 insertions(+), 5 deletions(-) > > diff --git a/linux-headers/asm-x86/kvm.h b/linux-headers/asm-x86/kvm.h > index d3a8778..855b3c7 100644 > --- a/linux-headers/asm-x86/kvm.h > +++ b/linux-headers/asm-x86/kvm.h > @@ -126,7 +126,10 @@ struct kvm_segment { > __u8 type; > __u8 present, dpl, db, s, l, g, avl; > __u8 unusable; > - __u8 padding; > + union { > + __u8 cpl; > + __u8 padding; > + }; > }; > > struct kvm_dtable { > diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h > index b278ab3..57aff89 100644 > --- a/linux-headers/linux/kvm.h > +++ b/linux-headers/linux/kvm.h > @@ -743,6 +743,9 @@ struct kvm_ppc_smmu_info { > #define KVM_CAP_IOAPIC_POLARITY_IGNORED 97 > #define KVM_CAP_ENABLE_CAP_VM 98 > #define KVM_CAP_S390_IRQCHIP 99 > +#define KVM_CAP_IOEVENTFD_NO_LENGTH 100 > +#define KVM_CAP_VM_ATTRIBUTES 101 > +#define KVM_CAP_X86_CPL 102 > > #ifdef KVM_CAP_IRQ_ROUTING > > diff --git a/target-i386/kvm.c b/target-i386/kvm.c > index 0d894ef..f78e7af 100644 > --- a/target-i386/kvm.c > +++ b/target-i386/kvm.c > @@ -722,6 +722,15 @@ int kvm_arch_init_vcpu(CPUState *cs) > env->kvm_xsave_buf = qemu_memalign(4096, sizeof(struct kvm_xsave)); > } > > + r = kvm_check_extension(cs->kvm_state, KVM_CAP_X86_CPL); > + if (r) { > + r = kvm_vm_enable_cap(cs->kvm_state, KVM_CAP_X86_CPL, 0); > + if (r < 0) { > + fprintf(stderr, "KVM_ENABLE_CAP failed\n"); > + return r; > + } > + } > + > return 0; > } > > @@ -1068,6 +1077,7 @@ static int kvm_put_xcrs(X86CPU *cpu) > static int kvm_put_sregs(X86CPU *cpu) > { > CPUX86State *env = &cpu->env; > + CPUState *cs = CPU(cpu); > struct kvm_sregs sregs; > > memset(sregs.interrupt_bitmap, 0, sizeof(sregs.interrupt_bitmap)); > @@ -1112,7 +1122,11 @@ static int kvm_put_sregs(X86CPU *cpu) > > sregs.efer = env->efer; > > - return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_SREGS, &sregs); > + if (kvm_check_extension(cs->kvm_state, KVM_CAP_X86_CPL)) { > + sregs.cs.cpl = env->hflags & HF_CPL_MASK; > + } > + > + return kvm_vcpu_ioctl(cs, KVM_SET_SREGS, &sregs); > } > > static void kvm_msr_entry_set(struct kvm_msr_entry *entry, > @@ -1380,11 +1394,12 @@ static int kvm_get_xcrs(X86CPU *cpu) > static int kvm_get_sregs(X86CPU *cpu) > { > CPUX86State *env = &cpu->env; > + CPUState *cs = CPU(cpu); > struct kvm_sregs sregs; > uint32_t hflags; > int bit, i, ret; > > - ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_SREGS, &sregs); > + ret = kvm_vcpu_ioctl(cs, KVM_GET_SREGS, &sregs); > if (ret < 0) { > return ret; > } > @@ -1430,7 +1445,11 @@ static int kvm_get_sregs(X86CPU *cpu) > HF_OSFXSR_MASK | HF_LMA_MASK | HF_CS32_MASK | \ > HF_SS32_MASK | HF_CS64_MASK | HF_ADDSEG_MASK) > > - hflags = (env->segs[R_CS].flags >> DESC_DPL_SHIFT) & HF_CPL_MASK; > + if (kvm_check_extension(cs->kvm_state, KVM_CAP_X86_CPL)) { > + hflags = sregs.cs.cpl; > + } else { > + hflags = (env->segs[R_CS].flags >> DESC_DPL_SHIFT) & HF_CPL_MASK; > + } > hflags |= (env->cr[0] & CR0_PE_MASK) << (HF_PE_SHIFT - CR0_PE_SHIFT); > hflags |= (env->cr[0] << (HF_MP_SHIFT - CR0_MP_SHIFT)) & > (HF_MP_MASK | HF_EM_MASK | HF_TS_MASK); > diff --git a/target-i386/machine.c b/target-i386/machine.c > index 168cab6..328856f 100644 > --- a/target-i386/machine.c > +++ b/target-i386/machine.c > @@ -284,7 +284,6 @@ static void cpu_pre_save(void *opaque) > env->segs[R_GS].flags &= ~(env->segs[R_GS].flags & DESC_DPL_MASK); > env->segs[R_SS].flags &= ~(env->segs[R_SS].flags & DESC_DPL_MASK); > } > - > } > > static int cpu_post_load(void *opaque, int version_id) > @@ -312,6 +311,23 @@ static int cpu_post_load(void *opaque, int version_id) > env->segs[R_SS].flags &= ~(env->segs[R_SS].flags & DESC_DPL_MASK); > } > > + /* Older versions of QEMU also blindly stored the DPL of CS into > + * hflags. Since we will use hflags to pass the CPL to KVM, fix > + * it up just in case it was wrong. > + * > + * There is still a small window where migration is broken, between > + * the time CR0.PE=1 and the subsequent long jump that reloads CS. > + * This cannot be fixed. > + */ > + if (!(env->cr[0] & CR0_PE_MASK)) { > + /* Force CPL=0 for real mode. */ > + env->hflags &= ~HF_CPL_MASK; > + } else if ((env->eflags & VM_MASK) && > + !(env->segs[R_CS].flags & DESC_L_MASK)) { > + /* Force CPL=3 for VM86 mode. */ > + env->hflags |= HF_CPL_MASK; > + } > + > /* XXX: restore FPU round state */ > env->fpstt = (env->fpus_vmstate >> 11) & 7; > env->fpus = env->fpus_vmstate & ~0x3800; >