[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] x86-64: emulation support for cmpxchg16b


  • To: Jan Beulich <jbeulich@xxxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxx>
  • From: Keir Fraser <keir.fraser@xxxxxxxxxxxxx>
  • Date: Tue, 22 Apr 2008 13:25:50 +0100
  • Delivery-date: Tue, 22 Apr 2008 05:26:32 -0700
  • List-id: Xen developer discussion <xen-devel.lists.xensource.com>
  • Thread-index: Acikc//OPkXb4BBnEd2NMQAX8io7RQ==
  • Thread-topic: [Xen-devel] [PATCH] x86-64: emulation support for cmpxchg16b

Thanks, I forgot about this one. Actually I will rework it so we only have
one cmpxchg() handling callback which takes pointers to old/new values and a
size up to 16 bytes.

 -- Keir

On 21/4/08 17:35, "Jan Beulich" <jbeulich@xxxxxxxxxx> wrote:

> With the x86 instruction emulator no pretty complete, I'd like to
> re-submit this patch to support cmpxchg16b on x86-64 and at once rename
> the underlying emulator callback function pointer (making clear that if
> implemented, it is to operate on two longs rather than two 32-bit
> values). At the same time it fixes an apparently wrong emulator context
> initialization in the shadow code.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxxxx>
> 
> Index: 2008-04-15/tools/tests/test_x86_emulator.c
> ===================================================================
> --- 2008-04-15.orig/tools/tests/test_x86_emulator.c 2008-04-01
> 11:10:21.000000000 +0200
> +++ 2008-04-15/tools/tests/test_x86_emulator.c 2008-04-15 08:48:15.000000000
> +0200
> @@ -74,7 +74,7 @@ static int cmpxchg(
>      return X86EMUL_OKAY;
>  }
>  
> -static int cmpxchg8b(
> +static int cmpxchg2(
>      unsigned int seg,
>      unsigned long offset,
>      unsigned long old_lo,
> @@ -94,7 +94,7 @@ static struct x86_emulate_ops emulops =
>      .insn_fetch = read,
>      .write      = write,
>      .cmpxchg    = cmpxchg,
> -    .cmpxchg8b  = cmpxchg8b
> +    .cmpxchg2   = cmpxchg2
>  };
>  
>  int main(int argc, char **argv)
> Index: 2008-04-15/tools/tests/x86_emulate.c
> ===================================================================
> --- 2008-04-15.orig/tools/tests/x86_emulate.c 2008-04-01 11:10:21.000000000
> +0200
> +++ 2008-04-15/tools/tests/x86_emulate.c 2008-04-15 08:48:15.000000000 +0200
> @@ -3,6 +3,8 @@
>  #include <string.h>
>  #include <public/xen.h>
>  
> +#define cpu_has_cmpxchg16b (sizeof(long) == 8)
> +
>  #include "x86_emulate/x86_emulate.h"
>  
>  #define __emulate_fpu_insn(_op)                 \
> Index: 2008-04-15/xen/arch/x86/mm.c
> ===================================================================
> --- 2008-04-15.orig/xen/arch/x86/mm.c 2008-04-11 14:48:16.000000000 +0200
> +++ 2008-04-15/xen/arch/x86/mm.c 2008-04-15 08:48:15.000000000 +0200
> @@ -3635,6 +3635,7 @@ static int ptwr_emulated_cmpxchg(
>          container_of(ctxt, struct ptwr_emulate_ctxt, ctxt));
>  }
>  
> +#ifdef __i386__
>  static int ptwr_emulated_cmpxchg8b(
>      enum x86_segment seg,
>      unsigned long offset,
> @@ -3650,13 +3651,16 @@ static int ptwr_emulated_cmpxchg8b(
>          offset, ((u64)old_hi << 32) | old, ((u64)new_hi << 32) | new, 8, 1,
>          container_of(ctxt, struct ptwr_emulate_ctxt, ctxt));
>  }
> +#else
> +#define ptwr_emulated_cmpxchg8b NULL
> +#endif
>  
>  static struct x86_emulate_ops ptwr_emulate_ops = {
>      .read       = ptwr_emulated_read,
>      .insn_fetch = ptwr_emulated_read,
>      .write      = ptwr_emulated_write,
>      .cmpxchg    = ptwr_emulated_cmpxchg,
> -    .cmpxchg8b  = ptwr_emulated_cmpxchg8b
> +    .cmpxchg2   = ptwr_emulated_cmpxchg8b
>  };
>  
>  /* Write page fault handler: check if guest is trying to modify a PTE. */
> Index: 2008-04-15/xen/arch/x86/mm/shadow/common.c
> ===================================================================
> --- 2008-04-15.orig/xen/arch/x86/mm/shadow/common.c 2008-04-11
> 14:48:16.000000000 +0200
> +++ 2008-04-15/xen/arch/x86/mm/shadow/common.c 2008-04-15 08:48:15.000000000
> +0200
> @@ -262,6 +262,7 @@ hvm_emulate_cmpxchg(enum x86_segment seg
>          v, addr, old, new, bytes, sh_ctxt);
>  }
>  
> +#ifdef __i386__
>  static int 
>  hvm_emulate_cmpxchg8b(enum x86_segment seg,
>                        unsigned long offset,
> @@ -288,13 +289,16 @@ hvm_emulate_cmpxchg8b(enum x86_segment s
>      return v->arch.paging.mode->shadow.x86_emulate_cmpxchg8b(
>          v, addr, old_lo, old_hi, new_lo, new_hi, sh_ctxt);
>  }
> +#else
> +#define hvm_emulate_cmpxchg8b NULL
> +#endif
>  
>  static struct x86_emulate_ops hvm_shadow_emulator_ops = {
>      .read       = hvm_emulate_read,
>      .insn_fetch = hvm_emulate_insn_fetch,
>      .write      = hvm_emulate_write,
>      .cmpxchg    = hvm_emulate_cmpxchg,
> -    .cmpxchg8b  = hvm_emulate_cmpxchg8b,
> +    .cmpxchg2   = hvm_emulate_cmpxchg8b,
>  };
>  
>  static int
> @@ -352,6 +356,7 @@ pv_emulate_cmpxchg(enum x86_segment seg,
>          v, offset, old, new, bytes, sh_ctxt);
>  }
>  
> +#ifdef __i386__
>  static int 
>  pv_emulate_cmpxchg8b(enum x86_segment seg,
>                       unsigned long offset,
> @@ -369,13 +374,16 @@ pv_emulate_cmpxchg8b(enum x86_segment se
>      return v->arch.paging.mode->shadow.x86_emulate_cmpxchg8b(
>          v, offset, old_lo, old_hi, new_lo, new_hi, sh_ctxt);
>  }
> +#else
> +#define pv_emulate_cmpxchg8b NULL
> +#endif
>  
>  static struct x86_emulate_ops pv_shadow_emulator_ops = {
>      .read       = pv_emulate_read,
>      .insn_fetch = pv_emulate_read,
>      .write      = pv_emulate_write,
>      .cmpxchg    = pv_emulate_cmpxchg,
> -    .cmpxchg8b  = pv_emulate_cmpxchg8b,
> +    .cmpxchg2   = pv_emulate_cmpxchg8b,
>  };
>  
>  struct x86_emulate_ops *shadow_init_emulation(
> @@ -390,7 +398,12 @@ struct x86_emulate_ops *shadow_init_emul
>  
>      if ( !is_hvm_vcpu(v) )
>      {
> +#ifndef CONFIG_COMPAT
>          sh_ctxt->ctxt.addr_size = sh_ctxt->ctxt.sp_size = BITS_PER_LONG;
> +#else
> +        sh_ctxt->ctxt.addr_size = sh_ctxt->ctxt.sp_size =
> +            !is_pv_32on64_vcpu(v) ? BITS_PER_LONG : COMPAT_BITS_PER_LONG;
> +#endif
>          return &pv_shadow_emulator_ops;
>      }
>  
> Index: 2008-04-15/xen/arch/x86/mm/shadow/multi.c
> ===================================================================
> --- 2008-04-15.orig/xen/arch/x86/mm/shadow/multi.c 2008-04-01
> 14:20:52.000000000 +0200
> +++ 2008-04-15/xen/arch/x86/mm/shadow/multi.c 2008-04-15 08:48:15.000000000
> +0200
> @@ -4432,7 +4432,8 @@ sh_x86_emulate_cmpxchg(struct vcpu *v, u
>      return rv;
>  }
>  
> -int
> +#ifdef __i386__
> +static int
>  sh_x86_emulate_cmpxchg8b(struct vcpu *v, unsigned long vaddr,
>                            unsigned long old_lo, unsigned long old_hi,
>                            unsigned long new_lo, unsigned long new_hi,
> @@ -4465,6 +4466,7 @@ sh_x86_emulate_cmpxchg8b(struct vcpu *v,
>      shadow_unlock(v->domain);
>      return rv;
>  }
> +#endif
>  
>  
>  /**************************************************************************/
> @@ -4738,7 +4740,9 @@ struct paging_mode sh_paging_mode = {
>      .shadow.detach_old_tables      = sh_detach_old_tables,
>      .shadow.x86_emulate_write      = sh_x86_emulate_write,
>      .shadow.x86_emulate_cmpxchg    = sh_x86_emulate_cmpxchg,
> +#ifdef __i386__
>      .shadow.x86_emulate_cmpxchg8b  = sh_x86_emulate_cmpxchg8b,
> +#endif
>      .shadow.make_monitor_table     = sh_make_monitor_table,
>      .shadow.destroy_monitor_table  = sh_destroy_monitor_table,
>  #if SHADOW_OPTIMIZATIONS & SHOPT_WRITABLE_HEURISTIC
> Index: 2008-04-15/xen/arch/x86/x86_emulate/x86_emulate.c
> ===================================================================
> --- 2008-04-15.orig/xen/arch/x86/x86_emulate/x86_emulate.c 2008-04-01
> 11:10:21.000000000 +0200
> +++ 2008-04-15/xen/arch/x86/x86_emulate/x86_emulate.c 2008-04-15
> 08:48:15.000000000 +0200
> @@ -3346,60 +3346,64 @@ x86_emulate(
>          src.val = x86_seg_gs;
>          goto pop_seg;
>  
> -    case 0xc7: /* Grp9 (cmpxchg8b) */
> -#if defined(__i386__)
> -    {
> -        unsigned long old_lo, old_hi;
> +    case 0xc7: /* Grp9 (cmpxchg{8,16}b) */
>          generate_exception_if((modrm_reg & 7) != 1, EXC_UD, -1);
>          generate_exception_if(ea.type != OP_MEM, EXC_UD, -1);
> -        if ( (rc = ops->read(ea.mem.seg, ea.mem.off+0, &old_lo, 4, ctxt)) ||
> -             (rc = ops->read(ea.mem.seg, ea.mem.off+4, &old_hi, 4, ctxt)) )
> -            goto done;
> -        if ( (old_lo != _regs.eax) || (old_hi != _regs.edx) )
> -        {
> -            _regs.eax = old_lo;
> -            _regs.edx = old_hi;
> -            _regs.eflags &= ~EFLG_ZF;
> -        }
> -        else if ( ops->cmpxchg8b == NULL )
> -        {
> -            rc = X86EMUL_UNHANDLEABLE;
> -            goto done;
> -        }
> -        else
> +#ifdef __x86_64__
> +        if ( op_bytes != 8 )
>          {
> -            if ( (rc = ops->cmpxchg8b(ea.mem.seg, ea.mem.off, old_lo, old_hi,
> -                                      _regs.ebx, _regs.ecx, ctxt)) != 0 )
> +            unsigned long old, new;
> +
> +            if ( (rc = ops->read(ea.mem.seg, ea.mem.off, &old, 8, ctxt)) != 0
> )
>                  goto done;
> -            _regs.eflags |= EFLG_ZF;
> -        }
> -        break;
> -    }
> -#elif defined(__x86_64__)
> -    {
> -        unsigned long old, new;
> -        generate_exception_if((modrm_reg & 7) != 1, EXC_UD, -1);
> -        generate_exception_if(ea.type != OP_MEM, EXC_UD, -1);
> -        if ( (rc = ops->read(ea.mem.seg, ea.mem.off, &old, 8, ctxt)) != 0 )
> -            goto done;
> -        if ( ((uint32_t)(old>>0) != (uint32_t)_regs.eax) ||
> -             ((uint32_t)(old>>32) != (uint32_t)_regs.edx) )
> -        {
> -            _regs.eax = (uint32_t)(old>>0);
> -            _regs.edx = (uint32_t)(old>>32);
> -            _regs.eflags &= ~EFLG_ZF;
> +            if ( ((uint32_t)(old>>0) != (uint32_t)_regs.eax) ||
> +                 ((uint32_t)(old>>32) != (uint32_t)_regs.edx) )
> +            {
> +                _regs.eax = (uint32_t)(old>>0);
> +                _regs.edx = (uint32_t)(old>>32);
> +                _regs.eflags &= ~EFLG_ZF;
> +            }
> +            else
> +            {
> +                new = (_regs.ecx<<32)|(uint32_t)_regs.ebx;
> +                if ( (rc = ops->cmpxchg(ea.mem.seg, ea.mem.off, old,
> +                                        new, 8, ctxt)) != 0 )
> +                    goto done;
> +                _regs.eflags |= EFLG_ZF;
> +            }
>          }
> +        else if ( !cpu_has_cmpxchg16b )
> +            generate_exception_if(1, EXC_UD, -1);
>          else
> +#endif
>          {
> -            new = (_regs.ecx<<32)|(uint32_t)_regs.ebx;
> -            if ( (rc = ops->cmpxchg(ea.mem.seg, ea.mem.off, old,
> -                                    new, 8, ctxt)) != 0 )
> +            unsigned long old_lo, old_hi;
> +
> +            if ( (rc = ops->read(ea.mem.seg, ea.mem.off,
> +                                 &old_lo, sizeof(old_lo), ctxt)) ||
> +                 (rc = ops->read(ea.mem.seg, ea.mem.off+sizeof(old_lo),
> +                                 &old_hi, sizeof(old_lo), ctxt)) )
> +                goto done;
> +            if ( (old_lo != _regs.eax) || (old_hi != _regs.edx) )
> +            {
> +                _regs.eax = old_lo;
> +                _regs.edx = old_hi;
> +                _regs.eflags &= ~EFLG_ZF;
> +            }
> +            else if ( ops->cmpxchg2 == NULL )
> +            {
> +                rc = X86EMUL_UNHANDLEABLE;
>                  goto done;
> -            _regs.eflags |= EFLG_ZF;
> +            }
> +            else
> +            {
> +                if ( (rc = ops->cmpxchg2(ea.mem.seg, ea.mem.off, old_lo,
> old_hi,
> +                                         _regs.ebx, _regs.ecx, ctxt)) != 0 )
> +                    goto done;
> +                _regs.eflags |= EFLG_ZF;
> +            }
>          }
>          break;
> -    }
> -#endif
>  
>      case 0xc8 ... 0xcf: /* bswap */
>          dst.type = OP_REG;
> @@ -3409,7 +3413,7 @@ x86_emulate(
>          {
>          default: /* case 2: */
>              /* Undefined behaviour. Writes zero on all tested CPUs. */
> -            dst.val = 0;
> +            __asm__("data16 bswap %k0" : "+r" (dst.val));
>              break;
>          case 4:
>  #ifdef __x86_64__
> Index: 2008-04-15/xen/arch/x86/x86_emulate/x86_emulate.h
> ===================================================================
> --- 2008-04-15.orig/xen/arch/x86/x86_emulate/x86_emulate.h 2008-04-01
> 11:10:21.000000000 +0200
> +++ 2008-04-15/xen/arch/x86/x86_emulate/x86_emulate.h 2008-04-15
> 08:48:15.000000000 +0200
> @@ -104,8 +104,9 @@ struct segment_register {
>   *     some out-of-band mechanism, unknown to the emulator. The memop signals
>   *     failure by returning X86EMUL_EXCEPTION to the emulator, which will
>   *     then immediately bail.
> - *  2. Valid access sizes are 1, 2, 4 and 8 bytes. On x86/32 systems only
> - *     cmpxchg8b_emulated need support 8-byte accesses.
> + *  2. Valid access sizes are 1, 2, 4, 8, and 16 bytes. On x86/32 systems
> only
> + *     cmpxchg2_emulated need support 8-byte accesses. On x86/64 systems only
> + *     cmpxchg2_emulated need support 16-byte accesses.
>   *  3. The emulator cannot handle 64-bit mode emulation on an x86/32 system.
>   */
>  struct x86_emulate_ops
> @@ -165,16 +166,17 @@ struct x86_emulate_ops
>          struct x86_emulate_ctxt *ctxt);
>  
>      /*
> -     * cmpxchg8b: Emulate an atomic (LOCKed) CMPXCHG8B operation.
> +     * cmpxchg2: Emulate an atomic (LOCKed) CMPXCHG{8,16}B operation.
>       *  @old:   [IN ] Value expected to be current at @addr.
>       *  @new:   [IN ] Value to write to @addr.
>       * NOTES:
> -     *  1. This function is only ever called when emulating a real CMPXCHG8B.
> -     *  2. This function is *never* called on x86/64 systems.
> -     *  2. Not defining this function (i.e., specifying NULL) is equivalent
> +     *  1. This function is only ever called when emulating a real
> CMPXCHG{8,16}B.
> +     *  2. This function is *never* called on x86/64 systems for emulating
> +     *     CMPXCHG8B.
> +     *  3. Not defining this function (i.e., specifying NULL) is equivalent
>       *     to defining a function that always returns X86EMUL_UNHANDLEABLE.
>       */
> -    int (*cmpxchg8b)(
> +    int (*cmpxchg2)(
>          enum x86_segment seg,
>          unsigned long offset,
>          unsigned long old_lo,
> Index: 2008-04-15/xen/arch/x86/x86_emulate.c
> ===================================================================
> --- 2008-04-15.orig/xen/arch/x86/x86_emulate.c 2008-04-01 14:20:52.000000000
> +0200
> +++ 2008-04-15/xen/arch/x86/x86_emulate.c 2008-04-15 08:48:15.000000000 +0200
> @@ -12,6 +12,8 @@
>  #include <asm/x86_emulate.h>
>  
>  #undef cmpxchg
> +#undef cpuid
> +#undef wbinvd
>  
>  #define __emulate_fpu_insn(_op)                 \
>  do{ int _exn;                                   \
> Index: 2008-04-15/xen/include/asm-x86/cpufeature.h
> ===================================================================
> --- 2008-04-15.orig/xen/include/asm-x86/cpufeature.h 2008-04-01
> 14:20:52.000000000 +0200
> +++ 2008-04-15/xen/include/asm-x86/cpufeature.h 2008-04-15 08:48:15.000000000
> +0200
> @@ -149,6 +149,7 @@
>  #define cpu_has_cyrix_arr boot_cpu_has(X86_FEATURE_CYRIX_ARR)
>  #define cpu_has_centaur_mcr boot_cpu_has(X86_FEATURE_CENTAUR_MCR)
>  #define cpu_has_clflush  boot_cpu_has(X86_FEATURE_CLFLSH)
> +#define cpu_has_cmpxchg16b 0
>  #define cpu_has_page1gb  0
>  #define cpu_has_efer  (boot_cpu_data.x86_capability[1] & 0x20100800)
>  #else /* __x86_64__ */
> @@ -175,6 +176,7 @@
>  #define cpu_has_cyrix_arr 0
>  #define cpu_has_centaur_mcr 0
>  #define cpu_has_clflush  boot_cpu_has(X86_FEATURE_CLFLSH)
> +#define cpu_has_cmpxchg16b boot_cpu_has(X86_FEATURE_CX16)
>  #define cpu_has_page1gb  boot_cpu_has(X86_FEATURE_PAGE1GB)
>  #define cpu_has_efer  1
>  #endif
> Index: 2008-04-15/xen/include/asm-x86/paging.h
> ===================================================================
> --- 2008-04-15.orig/xen/include/asm-x86/paging.h 2008-04-11 14:48:16.000000000
> +0200
> +++ 2008-04-15/xen/include/asm-x86/paging.h 2008-04-15 08:48:15.000000000
> +0200
> @@ -83,12 +83,14 @@ struct shadow_paging_mode {
>                                              unsigned long new,
>                                              unsigned int bytes,
>                                              struct sh_emulate_ctxt *sh_ctxt);
> +#ifdef __i386__
>      int           (*x86_emulate_cmpxchg8b )(struct vcpu *v, unsigned long va,
>                                              unsigned long old_lo,
>                                              unsigned long old_hi,
>                                              unsigned long new_lo,
>                                              unsigned long new_hi,
>                                              struct sh_emulate_ctxt *sh_ctxt);
> +#endif
>      mfn_t         (*make_monitor_table    )(struct vcpu *v);
>      void          (*destroy_monitor_table )(struct vcpu *v, mfn_t mmfn);
>      int           (*guess_wrmap           )(struct vcpu *v,
> Index: 2008-04-15/xen/include/asm-x86/x86_emulate.h
> ===================================================================
> --- 2008-04-15.orig/xen/include/asm-x86/x86_emulate.h 2008-04-01
> 14:20:52.000000000 +0200
> +++ 2008-04-15/xen/include/asm-x86/x86_emulate.h 2008-04-15 08:48:15.000000000
> +0200
> @@ -16,6 +16,7 @@
>  #include <xen/types.h>
>  #include <xen/lib.h>
>  #include <asm/regs.h>
> +#include <asm/processor.h>
>  
>  #include "../../arch/x86/x86_emulate/x86_emulate.h"
>  
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.