libipt, ild, insn: merge instruction decode and classification

Instruction decode provides a fine-grain classification in struct pt_ild that
is translated into a coarser instruction class and into a newly introduced
struct pt_insn_ext.

Merge this post-processing step into instruction decode.

Change-Id: I86d776937ed5dd83ea2fe9bb20992d27266619c3
Signed-off-by: Markus Metzger <markus.t.metzger@intel.com>
diff --git a/libipt/internal/include/pt_ild.h b/libipt/internal/include/pt_ild.h
index 1f76ea6..fc96d6a 100644
--- a/libipt/internal/include/pt_ild.h
+++ b/libipt/internal/include/pt_ild.h
@@ -52,30 +52,8 @@
 
 	/* outputs */
 	uint8_t length;	/* bytes */
-	pti_inst_enum_t iclass;
-	uint64_t direct_target;	/* if direct_indirect = 1 */
 	union {
 		struct {
-			uint32_t branch:1;	/* direct or indirect */
-
-			/* direct jmp, direct call or rel/direct branch sets
-			 * branch_direct = 1.
-			 *
-			 * 1=direct, 0=indirect
-			 */
-			uint32_t branch_direct:1;
-
-			/* this includes other transfers like SYSENTER,
-			 * SYSEXIT, and IRET.
-			 *
-			 * 1=far, 0=near
-			 */
-			uint32_t branch_far:1;
-
-			uint32_t ret:1;
-			uint32_t call:1;
-			uint32_t cond:1;
-			/* internal fields */
 			uint32_t osz:1;
 			uint32_t asz:1;
 			uint32_t lock:1;
@@ -136,6 +114,7 @@
  * Returns zero if a non-interesting instruction was encountered.
  * Returns a negative error code otherwise.
  */
-extern int pt_instruction_decode(struct pt_ild *ild);
+extern int pt_instruction_decode(struct pt_insn *insn, struct pt_insn_ext *iext,
+				 const struct pt_ild *ild);
 
 #endif /* PT_ILD_H */
diff --git a/libipt/src/pt_ild.c b/libipt/src/pt_ild.c
index c329b13..0d37e74 100644
--- a/libipt/src/pt_ild.c
+++ b/libipt/src/pt_ild.c
@@ -874,12 +874,12 @@
 	return x;
 }
 
-static int set_branch_target(struct pt_ild *ild)
+static int set_branch_target(struct pt_insn_ext *iext, const struct pt_ild *ild)
 {
 	int64_t npc;
 	uint64_t sign_extended_disp = 0;
 
-	if (!ild)
+	if (!iext || !ild)
 		return -pte_internal;
 
 	if (ild->disp_bytes == 1)
@@ -897,7 +897,9 @@
 		return -pte_bad_insn;
 
 	npc = (int64_t) (ild->runtime_address + ild->length);
-	ild->direct_target = (uint64_t) (npc + sign_extended_disp);
+
+	iext->variant.branch.is_direct = 1;
+	iext->variant.branch.target = (uint64_t) (npc + sign_extended_disp);
 
 	/* We return 1 to indicate an interesting instruction so our caller can
 	 * just forward the return value.
@@ -933,14 +935,18 @@
 	return decode(ild);
 }
 
-int pt_instruction_decode(struct pt_ild *ild)
+int pt_instruction_decode(struct pt_insn *insn, struct pt_insn_ext *iext,
+			  const struct pt_ild *ild)
 {
 	uint8_t opcode, map;
 
-	if (!ild)
+	if (!iext || !ild)
 		return -pte_internal;
 
-	ild->iclass = PTI_INST_INVALID;
+	iext->iclass = PTI_INST_INVALID;
+	memset(&iext->variant, 0, sizeof(iext->variant));
+
+	insn->iclass = ptic_other;
 
 	opcode = ild->nominal_opcode;
 	map = ild->map;
@@ -953,21 +959,19 @@
 	/* PTI_INST_JCC,   70...7F, 0F (0x80...0x8F) */
 	if (opcode >= 0x70 && opcode <= 0x7F) {
 		if (map == PTI_MAP_0) {
-			ild->iclass = PTI_INST_JCC;
-			ild->u.s.branch = 1;
-			ild->u.s.branch_direct = 1;
-			ild->u.s.cond = 1;
-			return set_branch_target(ild);
+			insn->iclass = ptic_cond_jump;
+			iext->iclass = PTI_INST_JCC;
+
+			return set_branch_target(iext, ild);
 		}
 		return 0;
 	}
 	if (opcode >= 0x80 && opcode <= 0x8F) {
 		if (map == PTI_MAP_1) {
-			ild->iclass = PTI_INST_JCC;
-			ild->u.s.branch = 1;
-			ild->u.s.branch_direct = 1;
-			ild->u.s.cond = 1;
-			return set_branch_target(ild);
+			insn->iclass = ptic_cond_jump;
+			iext->iclass = PTI_INST_JCC;
+
+			return set_branch_target(iext, ild);
 		}
 		return 0;
 	}
@@ -975,10 +979,8 @@
 	switch (ild->nominal_opcode) {
 	case 0x9A:
 		if (map == PTI_MAP_0) {
-			ild->iclass = PTI_INST_CALL_9A;
-			ild->u.s.branch = 1;
-			ild->u.s.branch_far = 1;
-			ild->u.s.call = 1;
+			insn->iclass = ptic_far_call;
+			iext->iclass = PTI_INST_CALL_9A;
 			return 1;
 		}
 		return 0;
@@ -988,24 +990,20 @@
 			uint8_t reg = pti_get_modrm_reg(ild);
 
 			if (reg == 2) {
-				ild->iclass = PTI_INST_CALL_FFr2;
-				ild->u.s.branch = 1;
-				ild->u.s.call = 1;
+				insn->iclass = ptic_call;
+				iext->iclass = PTI_INST_CALL_FFr2;
 				return 1;
 			} else if (reg == 3) {
-				ild->iclass = PTI_INST_CALL_FFr3;
-				ild->u.s.branch = 1;
-				ild->u.s.branch_far = 1;
-				ild->u.s.call = 1;
+				insn->iclass = ptic_far_call;
+				iext->iclass = PTI_INST_CALL_FFr3;
 				return 1;
 			} else if (reg == 4) {
-				ild->iclass = PTI_INST_JMP_FFr4;
-				ild->u.s.branch = 1;
+				insn->iclass = ptic_jump;
+				iext->iclass = PTI_INST_JMP_FFr4;
 				return 1;
 			} else if (reg == 5) {
-				ild->iclass = PTI_INST_JMP_FFr5;
-				ild->u.s.branch = 1;
-				ild->u.s.branch_far = 1;
+				insn->iclass = ptic_far_jump;
+				iext->iclass = PTI_INST_JMP_FFr5;
 				return 1;
 			}
 		}
@@ -1013,117 +1011,109 @@
 
 	case 0xE8:
 		if (map == PTI_MAP_0) {
-			ild->iclass = PTI_INST_CALL_E8;
-			ild->u.s.branch = 1;
-			ild->u.s.call = 1;
-			ild->u.s.branch_direct = 1;
-			return set_branch_target(ild);
+			insn->iclass = ptic_call;
+			iext->iclass = PTI_INST_CALL_E8;
+
+			return set_branch_target(iext, ild);
 		}
 		return 0;
 
 	case 0xCD:
 		if (map == PTI_MAP_0) {
-			ild->iclass = PTI_INST_INT;
+			iext->iclass = PTI_INST_INT;
 			return 1;
 		}
 		return 0;
 
 	case 0xCC:
 		if (map == PTI_MAP_0) {
-			ild->iclass = PTI_INST_INT3;
+			iext->iclass = PTI_INST_INT3;
 			return 1;
 		}
 		return 0;
 
 	case 0xCE:
 		if (map == PTI_MAP_0) {
-			ild->iclass = PTI_INST_INTO;
+			iext->iclass = PTI_INST_INTO;
 			return 1;
 		}
 		return 0;
 
 	case 0xF1:
 		if (map == PTI_MAP_0) {
-			ild->iclass = PTI_INST_INT1;
+			iext->iclass = PTI_INST_INT1;
 			return 1;
 		}
 		return 0;
 
 	case 0xCF:
 		if (map == PTI_MAP_0) {
-			ild->iclass = PTI_INST_IRET;
-			ild->u.s.branch = 1;
-			ild->u.s.branch_far = 1;
-			ild->u.s.ret = 1;
+			insn->iclass = ptic_far_return;
+			iext->iclass = PTI_INST_IRET;
 			return 1;
 		}
 		return 0;
 
 	case 0xE9:
 		if (map == PTI_MAP_0) {
-			ild->iclass = PTI_INST_JMP_E9;
-			ild->u.s.branch = 1;
-			ild->u.s.branch_direct = 1;
-			return set_branch_target(ild);
+			insn->iclass = ptic_jump;
+			iext->iclass = PTI_INST_JMP_E9;
+
+			return set_branch_target(iext, ild);
 		}
 		return 0;
 
 	case 0xEA:
 		if (map == PTI_MAP_0) {
-			ild->iclass = PTI_INST_JMP_EA;
-			ild->u.s.branch = 1;
-			ild->u.s.branch_far = 1;
-			/* FIXME: We do not set the branch target. */
+			/* Far jumps are treated as indirect jumps. */
+			insn->iclass = ptic_far_jump;
+			iext->iclass = PTI_INST_JMP_EA;
 			return 1;
 		}
 		return 0;
 
 	case 0xEB:
 		if (map == PTI_MAP_0) {
-			ild->iclass = PTI_INST_JMP_EB;
-			ild->u.s.branch = 1;
-			ild->u.s.branch_direct = 1;
-			return set_branch_target(ild);
-	}
+			insn->iclass = ptic_jump;
+			iext->iclass = PTI_INST_JMP_EB;
+
+			return set_branch_target(iext, ild);
+		}
 		return 0;
 
 	case 0xE3:
 		if (map == PTI_MAP_0) {
-			ild->iclass = PTI_INST_JrCXZ;
-			ild->u.s.branch = 1;
-			ild->u.s.branch_direct = 1;
-			ild->u.s.cond = 1;
-			return set_branch_target(ild);
+			insn->iclass = ptic_cond_jump;
+			iext->iclass = PTI_INST_JrCXZ;
+
+			return set_branch_target(iext, ild);
 		}
 		return 0;
 
 	case 0xE0:
 		if (map == PTI_MAP_0) {
-			ild->iclass = PTI_INST_LOOPNE;
-			ild->u.s.branch = 1;
-			ild->u.s.branch_direct = 1;
-			ild->u.s.cond = 1;
-			return set_branch_target(ild);
+			insn->iclass = ptic_cond_jump;
+			iext->iclass = PTI_INST_LOOPNE;
+
+			return set_branch_target(iext, ild);
 		}
 		return 0;
 
 	case 0xE1:
 		if (map == PTI_MAP_0) {
-			ild->iclass = PTI_INST_LOOPE;
-			ild->u.s.branch = 1;
-			ild->u.s.branch_direct = 1;
-			ild->u.s.cond = 1;
-			return set_branch_target(ild);
+			insn->iclass = ptic_cond_jump;
+			iext->iclass = PTI_INST_LOOPE;
+
+			return set_branch_target(iext, ild);
 		}
 		return 0;
 
 	case 0xE2:
 		if (map == PTI_MAP_0) {
-			ild->iclass = PTI_INST_LOOP;
-			ild->u.s.branch = 1;
-			ild->u.s.branch_direct = 1;
-			ild->u.s.cond = 1;
-			return set_branch_target(ild);
+			insn->iclass = ptic_cond_jump;
+			iext->iclass = PTI_INST_LOOP;
+
+			return set_branch_target(iext, ild);
 		}
 		return 0;
 
@@ -1131,85 +1121,71 @@
 		if (map == PTI_MAP_1)
 			if (pti_get_modrm_reg(ild) == 3)
 				if (!ild->u.s.rex_r) {
-					ild->iclass = PTI_INST_MOV_CR3;
+					iext->iclass = PTI_INST_MOV_CR3;
 					return 1;
 				}
 		return 0;
 
 	case 0xC3:
 		if (map == PTI_MAP_0) {
-			ild->iclass = PTI_INST_RET_C3;
-			ild->u.s.branch = 1;
-			ild->u.s.ret = 1;
+			insn->iclass = ptic_return;
+			iext->iclass = PTI_INST_RET_C3;
 			return 1;
 		}
 		return 0;
 
 	case 0xC2:
 		if (map == PTI_MAP_0) {
-			ild->iclass = PTI_INST_RET_C2;
-			ild->u.s.branch = 1;
-			ild->u.s.ret = 1;
+			insn->iclass = ptic_return;
+			iext->iclass = PTI_INST_RET_C2;
 			return 1;
 		}
 		return 0;
 
 	case 0xCB:
 		if (map == PTI_MAP_0) {
-			ild->iclass = PTI_INST_RET_CB;
-			ild->u.s.branch = 1;
-			ild->u.s.branch_far = 1;
-			ild->u.s.ret = 1;
+			insn->iclass = ptic_far_return;
+			iext->iclass = PTI_INST_RET_CB;
 			return 1;
 		}
 		return 0;
 
 	case 0xCA:
 		if (map == PTI_MAP_0) {
-			ild->iclass = PTI_INST_RET_CA;
-			ild->u.s.branch = 1;
-			ild->u.s.branch_far = 1;
-			ild->u.s.ret = 1;
+			insn->iclass = ptic_far_return;
+			iext->iclass = PTI_INST_RET_CA;
 			return 1;
 		}
 		return 0;
 
 	case 0x05:
 		if (map == PTI_MAP_1) {
-			ild->iclass = PTI_INST_SYSCALL;
-			ild->u.s.branch = 1;
-			ild->u.s.branch_far = 1;
-			ild->u.s.call = 1;
+			insn->iclass = ptic_far_call;
+			iext->iclass = PTI_INST_SYSCALL;
 			return 1;
 		}
 		return 0;
 
 	case 0x34:
 		if (map == PTI_MAP_1) {
-			ild->iclass = PTI_INST_SYSENTER;
-			ild->u.s.branch = 1;
-			ild->u.s.branch_far = 1;
-			ild->u.s.call = 1;
+			insn->iclass = ptic_far_call;
+			iext->iclass = PTI_INST_SYSENTER;
 			return 1;
 		}
 		return 0;
 
 	case 0x35:
 		if (map == PTI_MAP_1) {
-			ild->iclass = PTI_INST_SYSEXIT;
-			ild->u.s.branch = 1;
-			ild->u.s.branch_far = 1;
-			ild->u.s.ret = 1;
+			insn->iclass = ptic_far_return;
+			iext->iclass = PTI_INST_SYSEXIT;
 			return 1;
 		}
 		return 0;
 
 	case 0x07:
 		if (map == PTI_MAP_1) {
-			ild->iclass = PTI_INST_SYSRET;
-			ild->u.s.branch = 1;
-			ild->u.s.branch_far = 1;
-			ild->u.s.ret = 1;
+			insn->iclass = ptic_far_return;
+			iext->iclass = PTI_INST_SYSRET;
 			return 1;
 		}
 		return 0;
@@ -1218,28 +1194,22 @@
 		if (map == PTI_MAP_1) {
 			switch (ild->modrm_byte) {
 			case 0xc1:
-				ild->iclass = PTI_INST_VMCALL;
-				ild->u.s.branch = 1;
-				ild->u.s.branch_far = 1;
-				ild->u.s.ret = 1;
+				insn->iclass = ptic_far_return;
+				iext->iclass = PTI_INST_VMCALL;
 				return 1;
 
 			case 0xc2:
-				ild->iclass = PTI_INST_VMLAUNCH;
-				ild->u.s.branch = 1;
-				ild->u.s.branch_far = 1;
-				ild->u.s.call = 1;
+				insn->iclass = ptic_far_call;
+				iext->iclass = PTI_INST_VMLAUNCH;
 				return 1;
 
 			case 0xc3:
-				ild->iclass = PTI_INST_VMRESUME;
-				ild->u.s.branch = 1;
-				ild->u.s.branch_far = 1;
-				ild->u.s.call = 1;
+				insn->iclass = ptic_far_call;
+				iext->iclass = PTI_INST_VMRESUME;
 				return 1;
 
 			default:
-				return 0;
+				break;
 			}
 		}
 		return 0;
@@ -1248,7 +1218,7 @@
 		if (map == PTI_MAP_1 &&
 		    pti_get_modrm_mod(ild) != 3 &&
 		    pti_get_modrm_reg(ild) == 6) {
-			ild->iclass = PTI_INST_VMPTRLD;
+			iext->iclass = PTI_INST_VMPTRLD;
 			return 1;
 		}
 		return 0;
diff --git a/libipt/src/pt_insn_decoder.c b/libipt/src/pt_insn_decoder.c
index 0d3aebc..d7bc79d 100644
--- a/libipt/src/pt_insn_decoder.c
+++ b/libipt/src/pt_insn_decoder.c
@@ -229,26 +229,6 @@
 	return pt_qry_core_bus_ratio(&decoder->query, cbr);
 }
 
-static enum pt_insn_class pt_insn_classify(const struct pt_ild *ild)
-{
-	if (!ild)
-		return ptic_error;
-
-	if (!ild->u.s.branch)
-		return ptic_other;
-
-	if (ild->u.s.cond)
-		return ptic_cond_jump;
-
-	if (ild->u.s.call)
-		return ild->u.s.branch_far ? ptic_far_call : ptic_call;
-
-	if (ild->u.s.ret)
-		return ild->u.s.branch_far ? ptic_far_return : ptic_return;
-
-	return ild->u.s.branch_far ? ptic_far_jump : ptic_jump;
-}
-
 /* Decode and analyze one instruction.
  *
  * Decodes the instructruction at @decoder->ip into @insn and @iext and updates
@@ -261,7 +241,7 @@
 		       struct pt_insn_decoder *decoder)
 {
 	struct pt_ild *ild;
-	int errcode, relevant;
+	int errcode;
 	int size;
 
 	if (!insn || !iext || !decoder)
@@ -294,25 +274,7 @@
 
 	insn->size = ild->length;
 
-	relevant = pt_instruction_decode(ild);
-	if (!relevant)
-		insn->iclass = ptic_other;
-	else {
-		if (relevant < 0)
-			return relevant;
-
-		insn->iclass = pt_insn_classify(ild);
-	}
-
-	memset(iext, 0, sizeof(*iext));
-
-	iext->iclass = ild->iclass;
-	if (ild->u.s.branch_direct) {
-		iext->variant.branch.is_direct = 1;
-		iext->variant.branch.target = ild->direct_target;
-	}
-
-	return 0;
+	return pt_instruction_decode(insn, iext, ild);
 }
 
 /* Check whether @ip is ahead of us.
@@ -340,7 +302,7 @@
 	ild.runtime_address = decoder->ip;
 
 	while (ild.runtime_address != ip) {
-		int size, errcode, relevant;
+		int size, errcode;
 
 		if (!steps--)
 			return 0;
@@ -360,26 +322,13 @@
 		if (errcode < 0)
 			return 0;
 
-		relevant = pt_instruction_decode(&ild);
-		if (!relevant)
-			insn.iclass = ptic_other;
-		else {
-			if (relevant < 0)
-				return relevant;
-
-			insn.iclass = pt_insn_classify(&ild);
-		}
+		errcode = pt_instruction_decode(&insn, &iext, &ild);
+		if (errcode < 0)
+			return 0;
 
 		insn.ip = ild.runtime_address;
 		insn.size = ild.length;
 
-		memset(&iext, 0, sizeof(iext));
-		iext.iclass = ild.iclass;
-		if (ild.u.s.branch_direct) {
-			iext.variant.branch.is_direct = 1;
-			iext.variant.branch.target = ild.direct_target;
-		}
-
 		errcode = pt_insn_next_ip(&ild.runtime_address, &insn, &iext);
 		if (errcode < 0)
 			return 0;
@@ -711,14 +660,15 @@
 
 static int check_erratum_skd022(struct pt_insn_decoder *decoder)
 {
+	struct pt_insn_ext iext;
+	struct pt_insn insn;
 	struct pt_ild ild;
-	uint8_t raw[pt_max_insn_size];
 	int size, errcode, isid;
 
 	if (!decoder)
 		return -pte_internal;
 
-	size = pt_image_read(decoder->image, &isid, raw, sizeof(raw),
+	size = pt_image_read(decoder->image, &isid, insn.raw, sizeof(insn.raw),
 			     &decoder->asid, decoder->ip);
 	if (size < 0)
 		return 0;
@@ -727,18 +677,18 @@
 
 	ild.mode = decoder->mode;
 	ild.max_bytes = (uint8_t) size;
-	ild.itext = raw;
+	ild.itext = insn.raw;
 	ild.runtime_address = decoder->ip;
 
 	errcode = pt_instruction_length_decode(&ild);
 	if (errcode < 0)
 		return 0;
 
-	errcode = pt_instruction_decode(&ild);
+	errcode = pt_instruction_decode(&insn, &iext, &ild);
 	if (errcode < 0)
 		return 0;
 
-	switch (ild.iclass) {
+	switch (iext.iclass) {
 	default:
 		return 0;
 
diff --git a/libipt/test/src/ptunit-ild.c b/libipt/test/src/ptunit-ild.c
index 20713cb..1bbfb3a 100644
--- a/libipt/test/src/ptunit-ild.c
+++ b/libipt/test/src/ptunit-ild.c
@@ -49,7 +49,9 @@
  * Does not check whether the classification is correct.
  * This is left to the calling test.
  */
-static struct ptunit_result ptunit_ild_decode(struct pt_ild *ild,
+static struct ptunit_result ptunit_ild_decode(struct pt_insn *insn,
+					      struct pt_insn_ext *iext,
+					      struct pt_ild *ild,
 					      int interest, uint8_t size)
 {
 	int lret, dret;
@@ -58,7 +60,7 @@
 	ptu_int_eq(lret, 0);
 	ptu_uint_eq(ild->length, size);
 
-	dret = pt_instruction_decode(ild);
+	dret = pt_instruction_decode(insn, iext, ild);
 	ptu_int_eq(dret, interest);
 
 	return ptu_passed();
@@ -68,38 +70,48 @@
  *
  * We can't use a fixture since we don't know the instruction size upfront.
  */
-static void ptunit_ild_init(struct pt_ild *ild, uint8_t *insn,
+static void ptunit_ild_init(struct pt_ild *ild, uint8_t *raw,
 			    uint8_t size, enum pt_exec_mode mode)
 {
 	memset(ild, 0, sizeof(*ild));
-	ild->itext = insn;
+	ild->itext = raw;
 	ild->max_bytes = size;
 	ild->mode = mode;
 	ild->runtime_address = pti_addr;
 }
 
 /* Check that a boring instruction is decoded correctly. */
-static struct ptunit_result ptunit_ild_boring(uint8_t *insn, uint8_t size,
+static struct ptunit_result ptunit_ild_boring(uint8_t *raw, uint8_t size,
 					      enum pt_exec_mode mode)
 {
+	struct pt_insn_ext iext;
+	struct pt_insn insn;
 	struct pt_ild ild;
 
-	ptunit_ild_init(&ild, insn, size, mode);
-	ptu_test(ptunit_ild_decode, &ild, pti_boring, size);
+	ptunit_ild_init(&ild, raw, size, mode);
+	memset(&iext, 0, sizeof(iext));
+	memset(&insn, 0, sizeof(insn));
+
+	ptu_test(ptunit_ild_decode, &insn, &iext, &ild, pti_boring, size);
 
 	return ptu_passed();
 }
 
 /* Check that an interesting instruction is decoded and classified correctly. */
-static struct ptunit_result ptunit_ild_classify(uint8_t *insn, uint8_t size,
+static struct ptunit_result ptunit_ild_classify(uint8_t *raw, uint8_t size,
 						enum pt_exec_mode mode,
 						pti_inst_enum_t iclass)
 {
+	struct pt_insn_ext iext;
+	struct pt_insn insn;
 	struct pt_ild ild;
 
-	ptunit_ild_init(&ild, insn, size, mode);
-	ptu_test(ptunit_ild_decode, &ild, interesting, size);
-	ptu_int_eq(ild.iclass, iclass);
+	ptunit_ild_init(&ild, raw, size, mode);
+	memset(&iext, 0, sizeof(iext));
+	memset(&insn, 0, sizeof(insn));
+
+	ptu_test(ptunit_ild_decode, &insn, &iext, &ild, interesting, size);
+	ptu_int_eq(iext.iclass, iclass);
 
 	return ptu_passed();
 }