diff --git a/hypervisor/arch/x86/guest/instr_emul.c b/hypervisor/arch/x86/guest/instr_emul.c index 461839e0a..5bfd8903a 100644 --- a/hypervisor/arch/x86/guest/instr_emul.c +++ b/hypervisor/arch/x86/guest/instr_emul.c @@ -1598,7 +1598,7 @@ vie_calculate_gla(enum vm_cpu_mode cpu_mode, enum cpu_reg_name seg, uint8_t glasize; uint32_t type; - ASSERT(seg >= CPU_REG_ES && seg <= CPU_REG_GS, + ASSERT((seg >= CPU_REG_SEG_FIRST) && (seg <= CPU_REG_SEG_LAST), "%s: invalid segment %d", __func__, seg); ASSERT(length == 1U || length == 2U || length == 4U || length == 8U, "%s: invalid operand size %hhu", __func__, length); @@ -1725,10 +1725,6 @@ vie_init(struct vie *vie, struct vcpu *vcpu) (void)memset(vie, 0U, sizeof(struct vie)); - vie->base_register = CPU_REG_LAST; - vie->index_register = CPU_REG_LAST; - vie->segment_register = CPU_REG_LAST; - err_code = PAGE_FAULT_ID_FLAG; ret = copy_from_gva(vcpu, vie->inst, guest_rip_gva, inst_len, &err_code); @@ -2021,7 +2017,7 @@ decode_sib(struct vie *vie) } /* 'scale' makes sense only in the context of an index register */ - if (vie->index_register < CPU_REG_LAST) { + if (vie->index_register <= CPU_REG_LAST) { vie->scale = 1U << vie->ss; } diff --git a/hypervisor/arch/x86/guest/instr_emul_wrapper.c b/hypervisor/arch/x86/guest/instr_emul_wrapper.c index b49f858d5..0c7495a8a 100644 --- a/hypervisor/arch/x86/guest/instr_emul_wrapper.c +++ b/hypervisor/arch/x86/guest/instr_emul_wrapper.c @@ -32,21 +32,21 @@ int vm_get_register(struct vcpu *vcpu, enum cpu_reg_name reg, uint64_t *retval) return -EINVAL; } - if ((reg >= CPU_REG_LAST) || (reg < CPU_REG_RAX)) { + if ((reg > CPU_REG_LAST) || (reg < CPU_REG_FIRST)) { return -EINVAL; } - if ((reg >= CPU_REG_RAX) && (reg <= CPU_REG_RDI)) { + if ((reg >= CPU_REG_GENERAL_FIRST) && (reg <= CPU_REG_GENERAL_LAST)) { cur_context = &vcpu->arch_vcpu.contexts[vcpu->arch_vcpu.cur_context]; *retval = cur_context->guest_cpu_regs.longs[reg]; - } else if ((reg > CPU_REG_RDI) && (reg < CPU_REG_LAST)) { + } else if ((reg >= CPU_REG_NONGENERAL_FIRST) && (reg <= CPU_REG_NONGENERAL_LAST)) { uint32_t field = get_vmcs_field(reg); if (field != VMX_INVALID_VMCS_FIELD) { - if (reg < CPU_REG_NATURAL_LAST) { + if (reg <= CPU_REG_NATURAL_LAST) { *retval = exec_vmread(field); - } else if (reg < CPU_REG_64BIT_LAST) { + } else if (reg <= CPU_REG_64BIT_LAST) { *retval = exec_vmread64(field); } else { *retval = (uint64_t)exec_vmread16(field); @@ -67,19 +67,19 @@ int vm_set_register(struct vcpu *vcpu, enum cpu_reg_name reg, uint64_t val) return -EINVAL; } - if ((reg >= CPU_REG_LAST) || (reg < CPU_REG_RAX)) { + if ((reg > CPU_REG_LAST) || (reg < CPU_REG_FIRST)) { return -EINVAL; } - if ((reg >= CPU_REG_RAX) && (reg <= CPU_REG_RDI)) { + if ((reg >= CPU_REG_GENERAL_FIRST) && (reg <= CPU_REG_GENERAL_LAST)) { cur_context = &vcpu->arch_vcpu.contexts[vcpu->arch_vcpu.cur_context]; cur_context->guest_cpu_regs.longs[reg] = val; - } else if ((reg > CPU_REG_RDI) && (reg < CPU_REG_LAST)) { + } else if ((reg >= CPU_REG_NONGENERAL_FIRST) && (reg <= CPU_REG_NONGENERAL_LAST)) { uint32_t field = get_vmcs_field(reg); if (field != VMX_INVALID_VMCS_FIELD) { - if (reg < CPU_REG_NATURAL_LAST) { + if (reg <= CPU_REG_NATURAL_LAST) { exec_vmwrite(field, val); } else if (reg <= CPU_REG_64BIT_LAST) { exec_vmwrite64(field, val); @@ -242,9 +242,8 @@ encode_vmcs_seg_desc(enum cpu_reg_name seg, *the corresponding field index MACROs in VMCS. * *Post Condition: - *In the non-general register names group (CPU_REG_CR0~CPU_REG_LAST), - *for register names CPU_REG_CR2, CPU_REG_IDTR, CPU_REG_GDTR, - *CPU_REG_NATURAL_LAST, CPU_REG_64BIT_LAST and CPU_REG_LAST, + *In the non-general register names group (CPU_REG_CR0~CPU_REG_GDTR), + *for register names CPU_REG_CR2, CPU_REG_IDTR and CPU_REG_GDTR, *this function returns VMX_INVALID_VMCS_FIELD; *for other register names, it returns correspoding field index MACROs *in VMCS. diff --git a/hypervisor/arch/x86/guest/instr_emul_wrapper.h b/hypervisor/arch/x86/guest/instr_emul_wrapper.h index 7ff333369..5868c66c2 100644 --- a/hypervisor/arch/x86/guest/instr_emul_wrapper.h +++ b/hypervisor/arch/x86/guest/instr_emul_wrapper.h @@ -39,19 +39,8 @@ * Within the following groups,register name need to be * kept in order: * General register names group (CPU_REG_RAX~CPU_REG_RDI); - * Non general register names group (CPU_REG_CR0~CPU_REG_LAST); + * Non general register names group (CPU_REG_CR0~CPU_REG_GDTR); * Segement register names group (CPU_REG_ES~CPU_REG_GS). - * - * CPU_REG_NATURAL_LAST indicates in the non general register names - * group the register name (less than CPU_REG_NATURAL_last) is - * corresponds to the natural width field in VMCS; - * - * CPU_REG_64BIT_LAST indicates in the non general register names - * group the register name (less than CPU_REG_64BIT_LAST and more than - * CPU_REG_NATURAL_last) corresponds to the 64-bit field in VMCS. - * - * CPU_REG_LAST indicates the last register name. - * */ enum cpu_reg_name { CPU_REG_RAX, @@ -77,13 +66,13 @@ enum cpu_reg_name { CPU_REG_RSP, CPU_REG_RIP, CPU_REG_RFLAGS, - CPU_REG_NATURAL_LAST, + /*CPU_REG_NATURAL_LAST*/ CPU_REG_EFER, CPU_REG_PDPTE0, CPU_REG_PDPTE1, CPU_REG_PDPTE2, CPU_REG_PDPTE3, - CPU_REG_64BIT_LAST, + /*CPU_REG_64BIT_LAST,*/ CPU_REG_ES, CPU_REG_CS, CPU_REG_SS, @@ -93,10 +82,61 @@ enum cpu_reg_name { CPU_REG_LDTR, CPU_REG_TR, CPU_REG_IDTR, - CPU_REG_GDTR, - CPU_REG_LAST + CPU_REG_GDTR + /*CPU_REG_LAST*/ }; +/** + * Define the following MACRO to make range checking clear. + * + * CPU_REG_FIRST indicates the first register name, its value + * is the same as CPU_REG_RAX; + * CPU_REG_LAST indicates the last register name, its value is + * the same as CPU_REG_GDTR; + * + * CPU_REG_GENERAL_FIRST indicates the first general register name, + * its value is the same as CPU_REG_RAX; + * CPU_REG_GENERAL_LAST indicates the last general register name, + * its value is the same as CPU_REG_RDI; + * + * CPU_REG_NONGENERAL_FIRST indicates the first non general register + * name, its value is the same as CPU_REG_CR0; + * CPU_REG_NONGENERAL_LAST indicates the last non general register + * name, its value is the same as CPU_REG_GDTR; + * + * CPU_REG_NATURAL_FIRST indicates the first register name that + * is corresponds to the natural width field in VMCS, its value + * is the same as CPU_REG_CR0; + * CPU_REG_NATURAL_LAST indicates the last register name that + * is corresponds to the natural width field in VMCS, its value + * is the same as CPU_REG_RFLAGS; + * + * CPU_REG_64BIT_FIRST indicates the first register name that + * is corresponds to the 64 bit field in VMCS, its value + * is the same as CPU_REG_EFER; + * CPU_REG_64BIT_LAST indicates the last register name that + * is corresponds to the 64 bit field in VMCS, its value + * is the same as CPU_REG_PDPTE3; + * + * CPU_REG_SEG_FIRST indicates the first segement register name, + * its value is the same as CPU_REG_ES; + * CPU_REG_SEG_FIRST indicates the last segement register name, + * its value is the same as CPU_REG_GS + * + */ +#define CPU_REG_FIRST CPU_REG_RAX +#define CPU_REG_LAST CPU_REG_GDTR +#define CPU_REG_GENERAL_FIRST CPU_REG_RAX +#define CPU_REG_GENERAL_LAST CPU_REG_RDI +#define CPU_REG_NONGENERAL_FIRST CPU_REG_CR0 +#define CPU_REG_NONGENERAL_LAST CPU_REG_GDTR +#define CPU_REG_NATURAL_FIRST CPU_REG_CR0 +#define CPU_REG_NATURAL_LAST CPU_REG_RFLAGS +#define CPU_REG_64BIT_FIRST CPU_REG_EFER +#define CPU_REG_64BIT_LAST CPU_REG_PDPTE3 +#define CPU_REG_SEG_FIRST CPU_REG_ES +#define CPU_REG_SEG_LAST CPU_REG_GS + struct vie_op { uint8_t op_byte; /* actual opcode byte */ uint8_t op_type; /* type of operation (e.g. MOV) */