Re: [Qemu-devel] [PATCH] KVM: x86: get/set CPL separately from CS

[Paolo Bonzini <pbonzini@redhat.com>]

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 <pbonzini@redhat.com>

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;
>