ia64/xen-unstable

changeset 11575:a753630a6456

[HVM][VMX] Clean up and audit VMX uses of instruction-length
info field. Todo: use by mmio decoder needs to be removed.
Signed-off-by: Keir Fraser <keir@xensource.com>
author kfraser@localhost.localdomain
date Fri Sep 22 11:33:03 2006 +0100 (2006-09-22)
parents 140dff9d90dc
children ad22c711ccb7 840453c5dd9a
files xen/arch/x86/hvm/svm/svm.c xen/arch/x86/hvm/vmx/io.c xen/arch/x86/hvm/vmx/vmx.c xen/include/asm-x86/hvm/svm/emulate.h xen/include/asm-x86/hvm/vmx/vmx.h
line diff
     1.1 --- a/xen/arch/x86/hvm/svm/svm.c	Fri Sep 22 09:12:00 2006 +0100
     1.2 +++ b/xen/arch/x86/hvm/svm/svm.c	Fri Sep 22 11:33:03 2006 +0100
     1.3 @@ -1847,12 +1847,12 @@ static int svm_cr_access(struct vcpu *v,
     1.4         where the prefix lives later on */
     1.5      index = skip_prefix_bytes(buffer, sizeof(buffer));
     1.6      
     1.7 -    if (type == TYPE_MOV_TO_CR) 
     1.8 +    if ( type == TYPE_MOV_TO_CR )
     1.9      {
    1.10          inst_len = __get_instruction_length_from_list(
    1.11              vmcb, list_a, ARR_SIZE(list_a), &buffer[index], &match);
    1.12      }
    1.13 -    else
    1.14 +    else /* type == TYPE_MOV_FROM_CR */
    1.15      {
    1.16          inst_len = __get_instruction_length_from_list(
    1.17              vmcb, list_b, ARR_SIZE(list_b), &buffer[index], &match);
     2.1 --- a/xen/arch/x86/hvm/vmx/io.c	Fri Sep 22 09:12:00 2006 +0100
     2.2 +++ b/xen/arch/x86/hvm/vmx/io.c	Fri Sep 22 11:33:03 2006 +0100
     2.3 @@ -108,11 +108,17 @@ asmlinkage void vmx_intr_assist(void)
     2.4          return;
     2.5      }
     2.6  
     2.7 +    /* This could be moved earlier in the VMX resume sequence. */
     2.8      __vmread(IDT_VECTORING_INFO_FIELD, &idtv_info_field);
     2.9      if (unlikely(idtv_info_field & INTR_INFO_VALID_MASK)) {
    2.10          __vmwrite(VM_ENTRY_INTR_INFO_FIELD, idtv_info_field);
    2.11  
    2.12 -        __vmread(VM_EXIT_INSTRUCTION_LEN, &inst_len);
    2.13 +        /*
    2.14 +         * Safe: the length will only be interpreted for software exceptions
    2.15 +         * and interrupts. If we get here then delivery of some event caused a
    2.16 +         * fault, and this always results in defined VM_EXIT_INSTRUCTION_LEN.
    2.17 +         */
    2.18 +        __vmread(VM_EXIT_INSTRUCTION_LEN, &inst_len); /* Safe */
    2.19          __vmwrite(VM_ENTRY_INSTRUCTION_LEN, inst_len);
    2.20  
    2.21          if (unlikely(idtv_info_field & 0x800)) { /* valid error code */
     3.1 --- a/xen/arch/x86/hvm/vmx/vmx.c	Fri Sep 22 09:12:00 2006 +0100
     3.2 +++ b/xen/arch/x86/hvm/vmx/vmx.c	Fri Sep 22 11:33:03 2006 +0100
     3.3 @@ -597,7 +597,7 @@ static int vmx_instruction_length(struct
     3.4  {
     3.5      unsigned long inst_len;
     3.6  
     3.7 -    if (__vmread(VM_EXIT_INSTRUCTION_LEN, &inst_len))
     3.8 +    if ( __vmread(VM_EXIT_INSTRUCTION_LEN, &inst_len) ) /* XXX Unsafe XXX */
     3.9          return 0;
    3.10      return inst_len;
    3.11  }
    3.12 @@ -836,11 +836,16 @@ int start_vmx(void)
    3.13  
    3.14  /*
    3.15   * Not all cases receive valid value in the VM-exit instruction length field.
    3.16 + * Callers must know what they're doing!
    3.17   */
    3.18 -#define __get_instruction_length(len) \
    3.19 -    __vmread(VM_EXIT_INSTRUCTION_LEN, &(len)); \
    3.20 -     if ((len) < 1 || (len) > 15) \
    3.21 -        __hvm_bug(&regs);
    3.22 +static int __get_instruction_length(void)
    3.23 +{
    3.24 +    int len;
    3.25 +    __vmread(VM_EXIT_INSTRUCTION_LEN, &len); /* Safe: callers audited */
    3.26 +    if ( (len < 1) || (len > 15) )
    3.27 +        __hvm_bug(guest_cpu_user_regs());
    3.28 +    return len;
    3.29 +}
    3.30  
    3.31  static void inline __update_guest_eip(unsigned long inst_len)
    3.32  {
    3.33 @@ -1051,15 +1056,18 @@ static int check_for_null_selector(unsig
    3.34      int i, inst_len;
    3.35      int inst_copy_from_guest(unsigned char *, unsigned long, int);
    3.36  
    3.37 -    __vmread(VM_EXIT_INSTRUCTION_LEN, &inst_len);
    3.38 +    inst_len = __get_instruction_length(); /* Safe: INS/OUTS */
    3.39      memset(inst, 0, MAX_INST_LEN);
    3.40 -    if (inst_copy_from_guest(inst, eip, inst_len) != inst_len) {
    3.41 +    if ( inst_copy_from_guest(inst, eip, inst_len) != inst_len )
    3.42 +    {
    3.43          printf("check_for_null_selector: get guest instruction failed\n");
    3.44          domain_crash_synchronous();
    3.45      }
    3.46  
    3.47 -    for (i = 0; i < inst_len; i++) {
    3.48 -        switch (inst[i]) {
    3.49 +    for ( i = 0; i < inst_len; i++ )
    3.50 +    {
    3.51 +        switch ( inst[i] )
    3.52 +        {
    3.53          case 0xf3: /* REPZ */
    3.54          case 0xf2: /* REPNZ */
    3.55          case 0xf0: /* LOCK */
    3.56 @@ -1184,15 +1192,14 @@ static void vmx_io_instruction(unsigned 
    3.57      }
    3.58  }
    3.59  
    3.60 -int
    3.61 -vmx_world_save(struct vcpu *v, struct vmx_assist_context *c)
    3.62 +static int vmx_world_save(struct vcpu *v, struct vmx_assist_context *c)
    3.63  {
    3.64 -    unsigned long inst_len;
    3.65      int error = 0;
    3.66  
    3.67 -    error |= __vmread(VM_EXIT_INSTRUCTION_LEN, &inst_len);
    3.68 +    /* NB. Skip transition instruction. */
    3.69      error |= __vmread(GUEST_RIP, &c->eip);
    3.70 -    c->eip += inst_len; /* skip transition instruction */
    3.71 +    c->eip += __get_instruction_length(); /* Safe: MOV Cn, LMSW, CLTS */
    3.72 +
    3.73      error |= __vmread(GUEST_RSP, &c->esp);
    3.74      error |= __vmread(GUEST_RFLAGS, &c->eflags);
    3.75  
    3.76 @@ -1249,8 +1256,7 @@ vmx_world_save(struct vcpu *v, struct vm
    3.77      return !error;
    3.78  }
    3.79  
    3.80 -int
    3.81 -vmx_world_restore(struct vcpu *v, struct vmx_assist_context *c)
    3.82 +static int vmx_world_restore(struct vcpu *v, struct vmx_assist_context *c)
    3.83  {
    3.84      unsigned long mfn, old_cr4, old_base_mfn;
    3.85      int error = 0;
    3.86 @@ -1364,8 +1370,7 @@ vmx_world_restore(struct vcpu *v, struct
    3.87  
    3.88  enum { VMX_ASSIST_INVOKE = 0, VMX_ASSIST_RESTORE };
    3.89  
    3.90 -int
    3.91 -vmx_assist(struct vcpu *v, int mode)
    3.92 +static int vmx_assist(struct vcpu *v, int mode)
    3.93  {
    3.94      struct vmx_assist_context c;
    3.95      u32 magic;
    3.96 @@ -1408,8 +1413,8 @@ vmx_assist(struct vcpu *v, int mode)
    3.97          break;
    3.98  
    3.99          /*
   3.100 -         * Restore the VMXASSIST_OLD_CONTEXT that was saved by VMX_ASSIST_INVOKE
   3.101 -         * above.
   3.102 +         * Restore the VMXASSIST_OLD_CONTEXT that was saved by
   3.103 +         * VMX_ASSIST_INVOKE above.
   3.104           */
   3.105      case VMX_ASSIST_RESTORE:
   3.106          /* save the old context */
   3.107 @@ -1552,7 +1557,8 @@ static int vmx_set_cr0(unsigned long val
   3.108              }
   3.109          }
   3.110  
   3.111 -        if ( vmx_assist(v, VMX_ASSIST_INVOKE) ) {
   3.112 +        if ( vmx_assist(v, VMX_ASSIST_INVOKE) )
   3.113 +        {
   3.114              set_bit(VMX_CPU_STATE_ASSIST_ENABLED, &v->arch.hvm_vmx.cpu_state);
   3.115              __vmread(GUEST_RIP, &eip);
   3.116              HVM_DBG_LOG(DBG_LEVEL_1,
   3.117 @@ -1815,7 +1821,8 @@ static void mov_from_cr(int cr, int gp, 
   3.118      HVM_DBG_LOG(DBG_LEVEL_VMMU, "CR%d, value = %lx", cr, value);
   3.119  }
   3.120  
   3.121 -static int vmx_cr_access(unsigned long exit_qualification, struct cpu_user_regs *regs)
   3.122 +static int vmx_cr_access(unsigned long exit_qualification,
   3.123 +                         struct cpu_user_regs *regs)
   3.124  {
   3.125      unsigned int gp, cr;
   3.126      unsigned long value;
   3.127 @@ -2069,6 +2076,47 @@ void restore_cpu_user_regs(struct cpu_us
   3.128  }
   3.129  #endif
   3.130  
   3.131 +static void vmx_reflect_exception(struct vcpu *v)
   3.132 +{
   3.133 +    int error_code, intr_info, vector;
   3.134 +
   3.135 +    __vmread(VM_EXIT_INTR_INFO, &intr_info);
   3.136 +    vector = intr_info & 0xff;
   3.137 +    if ( intr_info & INTR_INFO_DELIVER_CODE_MASK )
   3.138 +        __vmread(VM_EXIT_INTR_ERROR_CODE, &error_code);
   3.139 +    else
   3.140 +        error_code = VMX_DELIVER_NO_ERROR_CODE;
   3.141 +
   3.142 +#ifndef NDEBUG
   3.143 +    {
   3.144 +        unsigned long rip;
   3.145 +
   3.146 +        __vmread(GUEST_RIP, &rip);
   3.147 +        HVM_DBG_LOG(DBG_LEVEL_1, "rip = %lx, error_code = %x",
   3.148 +                    rip, error_code);
   3.149 +    }
   3.150 +#endif /* NDEBUG */
   3.151 +
   3.152 +    /*
   3.153 +     * According to Intel Virtualization Technology Specification for
   3.154 +     * the IA-32 Intel Architecture (C97063-002 April 2005), section
   3.155 +     * 2.8.3, SW_EXCEPTION should be used for #BP and #OV, and
   3.156 +     * HW_EXCEPTION used for everything else.  The main difference
   3.157 +     * appears to be that for SW_EXCEPTION, the EIP/RIP is incremented
   3.158 +     * by VM_ENTER_INSTRUCTION_LEN bytes, whereas for HW_EXCEPTION,
   3.159 +     * it is not.
   3.160 +     */
   3.161 +    if ( (intr_info & INTR_INFO_INTR_TYPE_MASK) == INTR_TYPE_SW_EXCEPTION )
   3.162 +    {
   3.163 +        int ilen = __get_instruction_length(); /* Safe: software exception */
   3.164 +        vmx_inject_sw_exception(v, vector, ilen);
   3.165 +    }
   3.166 +    else
   3.167 +    {
   3.168 +        vmx_inject_hw_exception(v, vector, error_code);
   3.169 +    }
   3.170 +}
   3.171 +
   3.172  asmlinkage void vmx_vmexit_handler(struct cpu_user_regs regs)
   3.173  {
   3.174      unsigned int exit_reason;
   3.175 @@ -2116,7 +2164,8 @@ asmlinkage void vmx_vmexit_handler(struc
   3.176  
   3.177      TRACE_VMEXIT(0,exit_reason);
   3.178  
   3.179 -    switch ( exit_reason ) {
   3.180 +    switch ( exit_reason )
   3.181 +    {
   3.182      case EXIT_REASON_EXCEPTION_NMI:
   3.183      {
   3.184          /*
   3.185 @@ -2242,43 +2291,38 @@ asmlinkage void vmx_vmexit_handler(struc
   3.186          domain_crash_synchronous();
   3.187          break;
   3.188      case EXIT_REASON_CPUID:
   3.189 +        inst_len = __get_instruction_length(); /* Safe: CPUID */
   3.190 +        __update_guest_eip(inst_len);
   3.191          vmx_vmexit_do_cpuid(&regs);
   3.192 -        __get_instruction_length(inst_len);
   3.193 -        __update_guest_eip(inst_len);
   3.194          break;
   3.195      case EXIT_REASON_HLT:
   3.196 -        __get_instruction_length(inst_len);
   3.197 +        inst_len = __get_instruction_length(); /* Safe: HLT */
   3.198          __update_guest_eip(inst_len);
   3.199          vmx_vmexit_do_hlt();
   3.200          break;
   3.201      case EXIT_REASON_INVLPG:
   3.202      {
   3.203 -        unsigned long   va;
   3.204 -
   3.205 +        unsigned long va;
   3.206 +        inst_len = __get_instruction_length(); /* Safe: INVLPG */
   3.207 +        __update_guest_eip(inst_len);
   3.208          __vmread(EXIT_QUALIFICATION, &va);
   3.209          vmx_vmexit_do_invlpg(va);
   3.210 -        __get_instruction_length(inst_len);
   3.211 -        __update_guest_eip(inst_len);
   3.212          break;
   3.213      }
   3.214      case EXIT_REASON_VMCALL:
   3.215      {
   3.216 -        __get_instruction_length(inst_len);
   3.217 +        inst_len = __get_instruction_length(); /* Safe: VMCALL */
   3.218 +        __update_guest_eip(inst_len);
   3.219          __vmread(GUEST_RIP, &rip);
   3.220          __vmread(EXIT_QUALIFICATION, &exit_qualification);
   3.221 -
   3.222          hvm_do_hypercall(&regs);
   3.223 -        __update_guest_eip(inst_len);
   3.224          break;
   3.225      }
   3.226      case EXIT_REASON_CR_ACCESS:
   3.227      {
   3.228          __vmread(GUEST_RIP, &rip);
   3.229 -        __get_instruction_length(inst_len);
   3.230          __vmread(EXIT_QUALIFICATION, &exit_qualification);
   3.231 -
   3.232 -        HVM_DBG_LOG(DBG_LEVEL_1, "rip = %lx, inst_len =%lx, exit_qualification = %lx",
   3.233 -                    rip, inst_len, exit_qualification);
   3.234 +        inst_len = __get_instruction_length(); /* Safe: MOV Cn, LMSW, CLTS */
   3.235          if ( vmx_cr_access(exit_qualification, &regs) )
   3.236              __update_guest_eip(inst_len);
   3.237          TRACE_VMEXIT(3,regs.error_code);
   3.238 @@ -2291,19 +2335,19 @@ asmlinkage void vmx_vmexit_handler(struc
   3.239          break;
   3.240      case EXIT_REASON_IO_INSTRUCTION:
   3.241          __vmread(EXIT_QUALIFICATION, &exit_qualification);
   3.242 -        __get_instruction_length(inst_len);
   3.243 +        inst_len = __get_instruction_length(); /* Safe: IN, INS, OUT, OUTS */
   3.244          vmx_io_instruction(exit_qualification, inst_len);
   3.245          TRACE_VMEXIT(4,exit_qualification);
   3.246          break;
   3.247      case EXIT_REASON_MSR_READ:
   3.248 -        __get_instruction_length(inst_len);
   3.249 +        inst_len = __get_instruction_length(); /* Safe: RDMSR */
   3.250 +        __update_guest_eip(inst_len);
   3.251          vmx_do_msr_read(&regs);
   3.252 -        __update_guest_eip(inst_len);
   3.253          break;
   3.254      case EXIT_REASON_MSR_WRITE:
   3.255 +        inst_len = __get_instruction_length(); /* Safe: WRMSR */
   3.256 +        __update_guest_eip(inst_len);
   3.257          vmx_do_msr_write(&regs);
   3.258 -        __get_instruction_length(inst_len);
   3.259 -        __update_guest_eip(inst_len);
   3.260          break;
   3.261      case EXIT_REASON_MWAIT_INSTRUCTION:
   3.262      case EXIT_REASON_MONITOR_INSTRUCTION:
     4.1 --- a/xen/include/asm-x86/hvm/svm/emulate.h	Fri Sep 22 09:12:00 2006 +0100
     4.2 +++ b/xen/include/asm-x86/hvm/svm/emulate.h	Fri Sep 22 11:33:03 2006 +0100
     4.3 @@ -94,15 +94,14 @@ extern int __get_instruction_length_from
     4.4  static inline int __get_instruction_length(struct vmcb_struct *vmcb, 
     4.5          enum instruction_index instr, u8 *guest_eip_buf)
     4.6  {
     4.7 -    return __get_instruction_length_from_list(vmcb, &instr, 1, guest_eip_buf, 
     4.8 -            NULL);
     4.9 +    return __get_instruction_length_from_list(
    4.10 +        vmcb, &instr, 1, guest_eip_buf, NULL);
    4.11  }
    4.12  
    4.13  
    4.14  static inline unsigned int is_prefix(u8 opc)
    4.15  {
    4.16 -    switch(opc)
    4.17 -    {
    4.18 +    switch ( opc ) {
    4.19      case 0x66:
    4.20      case 0x67:
    4.21      case 0x2E:
    4.22 @@ -115,22 +114,7 @@ static inline unsigned int is_prefix(u8 
    4.23      case 0xF3:
    4.24      case 0xF2:
    4.25  #if __x86_64__
    4.26 -    case 0x40:
    4.27 -    case 0x41:
    4.28 -    case 0x42:
    4.29 -    case 0x43:
    4.30 -    case 0x44:
    4.31 -    case 0x45:
    4.32 -    case 0x46:
    4.33 -    case 0x47:
    4.34 -    case 0x48:
    4.35 -    case 0x49:
    4.36 -    case 0x4a:
    4.37 -    case 0x4b:
    4.38 -    case 0x4c:
    4.39 -    case 0x4d:
    4.40 -    case 0x4e:
    4.41 -    case 0x4f:
    4.42 +    case 0x40 ... 0x4f:
    4.43  #endif /* __x86_64__ */
    4.44          return 1;
    4.45      }
    4.46 @@ -141,15 +125,15 @@ static inline unsigned int is_prefix(u8 
    4.47  static inline int skip_prefix_bytes(u8 *buf, size_t size)
    4.48  {
    4.49      int index;
    4.50 -    for (index = 0; index < size && is_prefix(buf[index]); index ++)  
    4.51 -        /* do nothing */ ;
    4.52 +    for ( index = 0; index < size && is_prefix(buf[index]); index++ )
    4.53 +        continue;
    4.54      return index;
    4.55  }
    4.56  
    4.57  
    4.58  
    4.59 -static void inline __update_guest_eip(struct vmcb_struct *vmcb, 
    4.60 -        int inst_len) 
    4.61 +static void inline __update_guest_eip(
    4.62 +    struct vmcb_struct *vmcb, int inst_len) 
    4.63  {
    4.64      ASSERT(inst_len > 0);
    4.65      vmcb->rip += inst_len;
     5.1 --- a/xen/include/asm-x86/hvm/vmx/vmx.h	Fri Sep 22 09:12:00 2006 +0100
     5.2 +++ b/xen/include/asm-x86/hvm/vmx/vmx.h	Fri Sep 22 11:33:03 2006 +0100
     5.3 @@ -469,7 +469,7 @@ static inline void __vmx_inject_exceptio
     5.4      if ( error_code != VMX_DELIVER_NO_ERROR_CODE ) {
     5.5          __vmwrite(VM_ENTRY_EXCEPTION_ERROR_CODE, error_code);
     5.6          intr_fields |= INTR_INFO_DELIVER_CODE_MASK;
     5.7 -     }
     5.8 +    }
     5.9  
    5.10      if ( ilen )
    5.11        __vmwrite(VM_ENTRY_INSTRUCTION_LEN, ilen);
    5.12 @@ -499,40 +499,4 @@ static inline void vmx_inject_extint(str
    5.13      __vmwrite(GUEST_INTERRUPTIBILITY_INFO, 0);
    5.14  }
    5.15  
    5.16 -static inline void vmx_reflect_exception(struct vcpu *v)
    5.17 -{
    5.18 -    int error_code, intr_info, vector;
    5.19 -
    5.20 -    __vmread(VM_EXIT_INTR_INFO, &intr_info);
    5.21 -    vector = intr_info & 0xff;
    5.22 -    if ( intr_info & INTR_INFO_DELIVER_CODE_MASK )
    5.23 -        __vmread(VM_EXIT_INTR_ERROR_CODE, &error_code);
    5.24 -    else
    5.25 -        error_code = VMX_DELIVER_NO_ERROR_CODE;
    5.26 -
    5.27 -#ifndef NDEBUG
    5.28 -    {
    5.29 -        unsigned long rip;
    5.30 -
    5.31 -        __vmread(GUEST_RIP, &rip);
    5.32 -        HVM_DBG_LOG(DBG_LEVEL_1, "rip = %lx, error_code = %x",
    5.33 -                    rip, error_code);
    5.34 -    }
    5.35 -#endif /* NDEBUG */
    5.36 -
    5.37 -    /* According to Intel Virtualization Technology Specification for
    5.38 -       the IA-32 Intel Architecture (C97063-002 April 2005), section
    5.39 -       2.8.3, SW_EXCEPTION should be used for #BP and #OV, and
    5.40 -       HW_EXCPEPTION used for everything else.  The main difference
    5.41 -       appears to be that for SW_EXCEPTION, the EIP/RIP is incremented
    5.42 -       by VM_ENTER_INSTRUCTION_LEN bytes, whereas for HW_EXCEPTION,
    5.43 -       it is not.  */
    5.44 -    if ( (intr_info & INTR_INFO_INTR_TYPE_MASK) == INTR_TYPE_SW_EXCEPTION ) {
    5.45 -        int ilen;
    5.46 -        __vmread(VM_EXIT_INSTRUCTION_LEN, &ilen);
    5.47 -        vmx_inject_sw_exception(v, vector, ilen);
    5.48 -    } else
    5.49 -        vmx_inject_hw_exception(v, vector, error_code);
    5.50 -}
    5.51 -
    5.52  #endif /* __ASM_X86_HVM_VMX_VMX_H__ */