HV:INSTR_EMUL: Clean up CPU_reg_name

wuxyintel · lijinxia · commit e13c852c4bff · 2018-07-19T14:08:03.000+08:00
In the current hypervisor, there are many members of CPU_reg_name
used to check range and useless register names.

Define some CPU_REG_XX_FIRST and CPU_REG_XX_LAST MACROs to
make range checking clear;
Remove useless register names CPU_REG_XX_LAST in CPU_reg_name;
Update the related caller.

V1--&gt;V2:
	Update a mistake, replace second CPU_REG_SEG_FIRST
	with CPU_REG_SEG_LAST in ASSERT.
V2--&gt;V3:
	Add '()' for bool expression in ASSERT.

Signed-off-by: Xiangyang Wu &lt;xiangyang.wu@intel.com&gt;
Acked-by: Eddie Dong &lt;eddie.dong@intel.com&gt;
diff --git 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
@@ -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
@@ -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) */
