libipt: fix overflow handling

Overflow handling currently assumes that the tracing enable state does not
change.  This is wrong.  Fix it.

Change-Id: I33db90d867842c6b261f612455956d4c2156f4a7
Signed-off-by: Markus Metzger <[email protected]>

libipt/include/intel-pt.h

@@ -1211,6 +1211,10 @@ struct pt_event {
 		/** Event: overflow. */
 		struct {
 			/** The address at which tracing resumes after overflow.
+			 *
+			 * This field is not valid, if ip_suppressed is set.
+			 * In this case, the overflow resolved while tracing
+			 * was disabled.
 			 */
 			uint64_t ip;
 		} overflow;

libipt/internal/include/pt_decoder_function.h

@@ -59,7 +59,10 @@ enum pt_decoder_function_flag {
 	pdff_unknown	= 1 << 5,
 
 	/* The decoded packet contains timing information. */
-	pdff_timing	= 1 << 6
+	pdff_timing	= 1 << 6,
+
+	/* The decoded packet contains padding. */
+	pdff_pad	= 1 << 7
 };
 
 /* An Intel(R) Processor Trace decoder function. */

libipt/src/pt_decoder_function.c

@@ -44,7 +44,7 @@ const struct pt_decoder_function pt_decode_pad = {
 	/* .packet = */ pt_pkt_decode_pad,
 	/* .decode = */ pt_qry_decode_pad,
 	/* .header = */ pt_qry_decode_pad,
-	/* .flags =  */ 0
+	/* .flags =  */ pdff_pad
 };
 
 const struct pt_decoder_function pt_decode_psb = {
@@ -107,7 +107,7 @@ const struct pt_decoder_function pt_decode_ovf = {
 	/* .packet = */ pt_pkt_decode_ovf,
 	/* .decode = */ pt_qry_decode_ovf,
 	/* .header = */ NULL,
-	/* .flags =  */ pdff_psbend
+	/* .flags =  */ pdff_psbend | pdff_event
 };
 
 const struct pt_decoder_function pt_decode_mode = {

libipt/src/pt_insn_decoder.c

@@ -703,14 +703,36 @@ static int process_overflow_event(struct pt_insn_decoder *decoder,
 	if (!decoder->event_may_change_ip)
 		return 0;
 
+	/* We don't know the TSX state.  Let's assume we execute normally.
+	 *
+	 * We also don't know the execution mode.  Let's keep what we have
+	 * in case we don't get an update before we have to decode the next
+	 * instruction.
+	 */
+	decoder->speculative = 0;
+
 	/* Disable tracing if we don't have an IP. */
 	if (ev->ip_suppressed) {
-		decoder->enabled = 0;
-		return 1;
-	}
+		/* Indicate the overflow in case tracing was enabled before.
+		 *
+		 * If tracing was disabled, we're not really resyncing.
+		 */
+		if (decoder->enabled) {
+			decoder->enabled = 0;
 
-	decoder->ip = ev->variant.overflow.ip;
-	insn->resynced = 1;
+			/* We mark the instruction as resynced.  It won't be
+			 * returned unless we enable tracing again, in which
+			 * case this is the labeling we want.
+			 */
+			insn->resynced = 1;
+		}
+	} else {
+		/* Jump to the IP at which the overflow was resolved. */
+		decoder->ip = ev->variant.overflow.ip;
+		decoder->enabled = 1;
+
+		insn->resynced = 1;
+	}
 
 	return 1;
 }

libipt/src/pt_query_decoder.c

@@ -938,19 +938,6 @@ int pt_qry_decode_tip_pge(struct pt_query_decoder *decoder)
 			default:
 				return -pte_internal;
 
-			case ptev_overflow: {
-				uint64_t ip;
-				int errcode;
-
-				/* We can't afford a suppressed IP, here. */
-				errcode = pt_last_ip_query(&ip, &decoder->ip);
-				if (errcode < 0)
-					return -pte_bad_packet;
-
-				ev->variant.overflow.ip = ip;
-			}
-				break;
-
 			case ptev_exec_mode:
 				pt_qry_add_event_ip(ev,
 						    &ev->variant.exec_mode.ip,
@@ -1318,12 +1305,44 @@ static int pt_qry_process_pending_psb_events(struct pt_query_decoder *decoder)
 	return 1;
 }
 
+/* Processes packets as long as the packet's event flag matches @pdff.
+ *
+ * Returns zero on success; a negative error code otherwise.
+ */
+static int pt_qry_read_ahead_while(struct pt_query_decoder *decoder,
+				   uint32_t pdff)
+{
+	for (;;) {
+		const struct pt_decoder_function *dfun;
+		int errcode;
+
+		errcode = pt_df_fetch(&decoder->next, decoder->pos,
+				      &decoder->config);
+		if (errcode < 0)
+			return errcode;
+
+		dfun = decoder->next;
+		if (!dfun)
+			return -pte_internal;
+
+		if (!dfun->decode)
+			return -pte_bad_context;
+
+		if (!(dfun->flags & pdff))
+			return 0;
+
+		errcode = dfun->decode(decoder);
+		if (errcode < 0)
+			return errcode;
+	}
+}
+
 int pt_qry_decode_ovf(struct pt_query_decoder *decoder)
 {
+	const struct pt_decoder_function *dfun;
 	struct pt_event *ev;
 	struct pt_time time;
-	int status, enabled;
-	enum pt_event_binding evb;
+	int status, errcode;
 
 	status = pt_qry_process_pending_psb_events(decoder);
 	if (status < 0)
@@ -1333,39 +1352,67 @@ int pt_qry_decode_ovf(struct pt_query_decoder *decoder)
 	if (status)
 		return 0;
 
-	/* Reset the decoder state - but preserve trace enabling and timing.
-	 *
-	 * We don't know how many packets we lost so any queued events or unused
-	 * TNT bits will likely be wrong.
-	 */
-	enabled = decoder->enabled;
+	/* Reset the decoder state but preserve timing. */
 	time = decoder->time;
-
 	pt_qry_reset(decoder);
-
-	if (enabled)
-		decoder->enabled = 1;
 	decoder->time = time;
 
-	/* OVF binds to FUP as long as tracing is enabled.
-	 * It binds to TIP.PGE when tracing is disabled.
-	 */
-	evb = enabled ? evb_fup : evb_tip;
+	/* We must consume the OVF before we search for the binding packet. */
+	decoder->pos += ptps_ovf;
 
-	/* Queue the overflow event.
+	/* Overflow binds to either FUP or TIP.PGE.
+	 *
+	 * If the overflow can be resolved while PacketEn=1 it binds to FUP.  We
+	 * can see timing packets between OVF anf FUP but that's it.
 	 *
-	 * We must be able to enqueue the overflow event since we just reset
-	 * the decoder state.
+	 * Otherwise, PacketEn will be zero when the overflow resolves and OVF
+	 * binds to TIP.PGE.  There can be packets between OVF and TIP.PGE that
+	 * do not depend on PacketEn.
+	 *
+	 * We don't need to decode everything until TIP.PGE, however.  As soon
+	 * as we see a non-timing non-FUP packet, we know that tracing has been
+	 * disabled before the overflow resolves.
 	 */
-	ev = pt_evq_enqueue(&decoder->evq, evb);
-	if (!ev)
-		return -pte_internal;
+	errcode = pt_qry_read_ahead_while(decoder, pdff_timing | pdff_pad);
+	if (errcode < 0) {
+		if (errcode != -pte_eos)
+			return errcode;
 
-	ev->type = ptev_overflow;
+		dfun = NULL;
+	} else {
+		dfun = decoder->next;
+		if (!dfun)
+			return -pte_internal;
+	}
+
+	if (dfun && (dfun->flags & pdff_fup)) {
+		ev = pt_evq_enqueue(&decoder->evq, evb_fup);
+		if (!ev)
+			return -pte_internal;
+
+		ev->type = ptev_overflow;
+
+		/* We set tracing to disabled in pt_qry_reset(); fix it. */
+		decoder->enabled = 1;
+	} else {
+		ev = pt_evq_standalone(&decoder->evq);
+		if (!ev)
+			return -pte_internal;
+
+		ev->type = ptev_overflow;
+
+		/* We suppress the IP to indicate that tracing has been
+		 * disabled before the overflow resolved.  There can be
+		 * several events before tracing is enabled again.
+		 */
+		ev->ip_suppressed = 1;
+
+		/* Publish the event. */
+		decoder->event = ev;
+	}
 
 	pt_qry_add_event_time(ev, decoder);
 
-	decoder->pos += ptps_ovf;
 	return 0;
 }
 

libipt/test/src/ptunit-query.c

@@ -1214,7 +1214,18 @@ event_overflow_tip_pge(struct ptu_decoder_fixture *dfix,
 	pt_encode_tip_pge(encoder, packet.ip, packet.ipc);
 
 	ptu_check(ptu_sync_decoder, decoder);
-	decoder->enabled = 0;
+
+	errcode = pt_qry_event(decoder, &event, sizeof(event));
+	ptu_int_eq(errcode, pts_event_pending);
+	ptu_int_eq(event.type, ptev_overflow);
+	ptu_uint_ne(event.ip_suppressed, 0);
+
+	if (!tsc)
+		ptu_int_eq(event.has_tsc, 0);
+	else {
+		ptu_int_eq(event.has_tsc, 1);
+		ptu_uint_eq(event.tsc, tsc);
+	}
 
 	errcode = pt_qry_event(decoder, &event, sizeof(event));
 	switch (ipc) {
@@ -1225,7 +1236,7 @@ event_overflow_tip_pge(struct ptu_decoder_fixture *dfix,
 	case pt_ipc_update_16:
 	case pt_ipc_update_32:
 	case pt_ipc_sext_48:
-		ptu_int_eq(errcode, pts_event_pending);
+		ptu_int_eq(errcode, pts_eos);
 		ptu_int_eq(event.type, ptev_enabled);
 		ptu_uint_eq(event.variant.enabled.ip, dfix->last_ip.ip);
 
@@ -1235,44 +1246,12 @@ event_overflow_tip_pge(struct ptu_decoder_fixture *dfix,
 			ptu_int_eq(event.has_tsc, 1);
 			ptu_uint_eq(event.tsc, tsc);
 		}
-
-		errcode = pt_qry_event(decoder, &event, sizeof(event));
-		ptu_int_eq(errcode, pts_eos);
-		ptu_int_eq(event.type, ptev_overflow);
-		ptu_uint_eq(event.variant.overflow.ip, dfix->last_ip.ip);
-
-		if (!tsc)
-			ptu_int_eq(event.has_tsc, 0);
-		else {
-			ptu_int_eq(event.has_tsc, 1);
-			ptu_uint_eq(event.tsc, tsc);
-		}
+		break;
 	}
 
 	return ptu_passed();
 }
 
-static struct ptunit_result
-event_overflow_tip_pge_cutoff_fail(struct ptu_decoder_fixture *dfix)
-{
-	struct pt_query_decoder *decoder = &dfix->decoder;
-	struct pt_encoder *encoder = &dfix->encoder;
-	struct pt_event event;
-	int errcode;
-
-	pt_encode_ovf(encoder);
-	pt_encode_tip_pge(encoder, 0, pt_ipc_update_32);
-
-	ptu_check(cutoff, decoder, encoder);
-	ptu_check(ptu_sync_decoder, decoder);
-	decoder->enabled = 0;
-
-	errcode = pt_qry_event(decoder, &event, sizeof(event));
-	ptu_int_eq(errcode, -pte_eos);
-
-	return ptu_passed();
-}
-
 static struct ptunit_result
 event_overflow_cutoff_fail(struct ptu_decoder_fixture *dfix)
 {
@@ -2238,7 +2217,6 @@ int main(int argc, char **argv)
 		   0);
 	ptu_run_fp(suite, event_overflow_tip_pge, dfix_empty, pt_ipc_sext_48,
 		   0);
-	ptu_run_f(suite, event_overflow_tip_pge_cutoff_fail, dfix_empty);
 	ptu_run_f(suite, event_overflow_cutoff_fail, dfix_empty);
 	ptu_run_fp(suite, event_stop, dfix_empty, 0);
 	ptu_run_fp(suite, event_exec_mode_tip, dfix_empty, pt_ipc_suppressed,
@@ -2339,7 +2317,6 @@ int main(int argc, char **argv)
 		   0x1000);
 	ptu_run_fp(suite, event_overflow_tip_pge, dfix_event, pt_ipc_sext_48,
 		   0x1000);
-	ptu_run_f(suite, event_overflow_tip_pge_cutoff_fail, dfix_event);
 	ptu_run_f(suite, event_overflow_cutoff_fail, dfix_event);
 	ptu_run_fp(suite, event_stop, dfix_event, 0x1000);
 	ptu_run_fp(suite, event_exec_mode_tip, dfix_event, pt_ipc_suppressed,