All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] perf intel-pt: Fix ip compression
@ 2016-07-20  9:00 Adrian Hunter
  2016-08-10  8:32 ` Adrian Hunter
  2016-08-16 18:13 ` [tip:perf/urgent] " tip-bot for Adrian Hunter
  0 siblings, 2 replies; 4+ messages in thread
From: Adrian Hunter @ 2016-07-20  9:00 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: Jiri Olsa, linux-kernel

June 2015 Intel SDM introduced IP Compression types 4 and 6. Refer section
36.4.2.2 Target IP (TIP) Packet - IP Compression.

Existing Intel PT packet decoder did not support type 4, and got type 6
wrong.  Because type 3 and type 4 have the same number of bytes, the packet
'count' has been changed from being the number of ip bytes to being the
type code.  That allows the Intel PT decoder to correctly decide whether to
sign-extend or use the last ip.  However that also meant the code had to be
adjusted in a number of places.

Currently hardware is not using the new compression types, so this fix has
no effect on existing hardware.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 .../perf/util/intel-pt-decoder/intel-pt-decoder.c  | 44 +++++++++++-----------
 .../util/intel-pt-decoder/intel-pt-pkt-decoder.c   | 24 ++++++++----
 2 files changed, 40 insertions(+), 28 deletions(-)

diff --git a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
index 9c8f15da86ce..8ff6c6a61291 100644
--- a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
+++ b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
@@ -123,8 +123,6 @@ struct intel_pt_decoder {
 	bool have_calc_cyc_to_tsc;
 	int exec_mode;
 	unsigned int insn_bytes;
-	uint64_t sign_bit;
-	uint64_t sign_bits;
 	uint64_t period;
 	enum intel_pt_period_type period_type;
 	uint64_t tot_insn_cnt;
@@ -191,9 +189,6 @@ struct intel_pt_decoder *intel_pt_decoder_new(struct intel_pt_params *params)
 	decoder->data               = params->data;
 	decoder->return_compression = params->return_compression;
 
-	decoder->sign_bit           = (uint64_t)1 << 47;
-	decoder->sign_bits          = ~(((uint64_t)1 << 48) - 1);
-
 	decoder->period             = params->period;
 	decoder->period_type        = params->period_type;
 
@@ -362,21 +357,30 @@ int intel_pt__strerror(int code, char *buf, size_t buflen)
 	return 0;
 }
 
-static uint64_t intel_pt_calc_ip(struct intel_pt_decoder *decoder,
-				 const struct intel_pt_pkt *packet,
+static uint64_t intel_pt_calc_ip(const struct intel_pt_pkt *packet,
 				 uint64_t last_ip)
 {
 	uint64_t ip;
 
 	switch (packet->count) {
-	case 2:
+	case 1:
 		ip = (last_ip & (uint64_t)0xffffffffffff0000ULL) |
 		     packet->payload;
 		break;
-	case 4:
+	case 2:
 		ip = (last_ip & (uint64_t)0xffffffff00000000ULL) |
 		     packet->payload;
 		break;
+	case 3:
+		ip = packet->payload;
+		/* Sign-extend 6-byte ip */
+		if (ip & (uint64_t)0x800000000000ULL)
+			ip |= (uint64_t)0xffff000000000000ULL;
+		break;
+	case 4:
+		ip = (last_ip & (uint64_t)0xffff000000000000ULL) |
+		     packet->payload;
+		break;
 	case 6:
 		ip = packet->payload;
 		break;
@@ -384,16 +388,12 @@ static uint64_t intel_pt_calc_ip(struct intel_pt_decoder *decoder,
 		return 0;
 	}
 
-	if (ip & decoder->sign_bit)
-		return ip | decoder->sign_bits;
-
 	return ip;
 }
 
 static inline void intel_pt_set_last_ip(struct intel_pt_decoder *decoder)
 {
-	decoder->last_ip = intel_pt_calc_ip(decoder, &decoder->packet,
-					    decoder->last_ip);
+	decoder->last_ip = intel_pt_calc_ip(&decoder->packet, decoder->last_ip);
 }
 
 static inline void intel_pt_set_ip(struct intel_pt_decoder *decoder)
@@ -1657,6 +1657,12 @@ next:
 	}
 }
 
+static inline bool intel_pt_have_ip(struct intel_pt_decoder *decoder)
+{
+	return decoder->last_ip || decoder->packet.count == 0 ||
+	       decoder->packet.count == 3 || decoder->packet.count == 6;
+}
+
 /* Walk PSB+ packets to get in sync. */
 static int intel_pt_walk_psb(struct intel_pt_decoder *decoder)
 {
@@ -1677,8 +1683,7 @@ static int intel_pt_walk_psb(struct intel_pt_decoder *decoder)
 
 		case INTEL_PT_FUP:
 			decoder->pge = true;
-			if (decoder->last_ip || decoder->packet.count == 6 ||
-			    decoder->packet.count == 0) {
+			if (intel_pt_have_ip(decoder)) {
 				uint64_t current_ip = decoder->ip;
 
 				intel_pt_set_ip(decoder);
@@ -1767,8 +1772,7 @@ static int intel_pt_walk_to_ip(struct intel_pt_decoder *decoder)
 		case INTEL_PT_TIP_PGE:
 		case INTEL_PT_TIP:
 			decoder->pge = decoder->packet.type != INTEL_PT_TIP_PGD;
-			if (decoder->last_ip || decoder->packet.count == 6 ||
-			    decoder->packet.count == 0)
+			if (intel_pt_have_ip(decoder))
 				intel_pt_set_ip(decoder);
 			if (decoder->ip)
 				return 0;
@@ -1776,9 +1780,7 @@ static int intel_pt_walk_to_ip(struct intel_pt_decoder *decoder)
 
 		case INTEL_PT_FUP:
 			if (decoder->overflow) {
-				if (decoder->last_ip ||
-				    decoder->packet.count == 6 ||
-				    decoder->packet.count == 0)
+				if (intel_pt_have_ip(decoder))
 					intel_pt_set_ip(decoder);
 				if (decoder->ip)
 					return 0;
diff --git a/tools/perf/util/intel-pt-decoder/intel-pt-pkt-decoder.c b/tools/perf/util/intel-pt-decoder/intel-pt-pkt-decoder.c
index b1257c816310..4f7b32020487 100644
--- a/tools/perf/util/intel-pt-decoder/intel-pt-pkt-decoder.c
+++ b/tools/perf/util/intel-pt-decoder/intel-pt-pkt-decoder.c
@@ -292,36 +292,46 @@ static int intel_pt_get_ip(enum intel_pt_pkt_type type, unsigned int byte,
 			   const unsigned char *buf, size_t len,
 			   struct intel_pt_pkt *packet)
 {
-	switch (byte >> 5) {
+	int ip_len;
+
+	packet->count = byte >> 5;
+
+	switch (packet->count) {
 	case 0:
-		packet->count = 0;
+		ip_len = 0;
 		break;
 	case 1:
 		if (len < 3)
 			return INTEL_PT_NEED_MORE_BYTES;
-		packet->count = 2;
+		ip_len = 2;
 		packet->payload = le16_to_cpu(*(uint16_t *)(buf + 1));
 		break;
 	case 2:
 		if (len < 5)
 			return INTEL_PT_NEED_MORE_BYTES;
-		packet->count = 4;
+		ip_len = 4;
 		packet->payload = le32_to_cpu(*(uint32_t *)(buf + 1));
 		break;
 	case 3:
-	case 6:
+	case 4:
 		if (len < 7)
 			return INTEL_PT_NEED_MORE_BYTES;
-		packet->count = 6;
+		ip_len = 6;
 		memcpy_le64(&packet->payload, buf + 1, 6);
 		break;
+	case 6:
+		if (len < 9)
+			return INTEL_PT_NEED_MORE_BYTES;
+		ip_len = 8;
+		packet->payload = le64_to_cpu(*(uint64_t *)(buf + 1));
+		break;
 	default:
 		return INTEL_PT_BAD_PACKET;
 	}
 
 	packet->type = type;
 
-	return packet->count + 1;
+	return ip_len + 1;
 }
 
 static int intel_pt_get_mode(const unsigned char *buf, size_t len,
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] perf intel-pt: Fix ip compression
  2016-07-20  9:00 [PATCH] perf intel-pt: Fix ip compression Adrian Hunter
@ 2016-08-10  8:32 ` Adrian Hunter
  2016-08-10 13:05   ` Arnaldo Carvalho de Melo
  2016-08-16 18:13 ` [tip:perf/urgent] " tip-bot for Adrian Hunter
  1 sibling, 1 reply; 4+ messages in thread
From: Adrian Hunter @ 2016-08-10  8:32 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: Jiri Olsa, linux-kernel

On 20/07/16 12:00, Adrian Hunter wrote:
> June 2015 Intel SDM introduced IP Compression types 4 and 6. Refer section
> 36.4.2.2 Target IP (TIP) Packet - IP Compression.
> 
> Existing Intel PT packet decoder did not support type 4, and got type 6
> wrong.  Because type 3 and type 4 have the same number of bytes, the packet
> 'count' has been changed from being the number of ip bytes to being the
> type code.  That allows the Intel PT decoder to correctly decide whether to
> sign-extend or use the last ip.  However that also meant the code had to be
> adjusted in a number of places.
> 
> Currently hardware is not using the new compression types, so this fix has
> no effect on existing hardware.
> 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>  .../perf/util/intel-pt-decoder/intel-pt-decoder.c  | 44 +++++++++++-----------
>  .../util/intel-pt-decoder/intel-pt-pkt-decoder.c   | 24 ++++++++----
>  2 files changed, 40 insertions(+), 28 deletions(-)

Hi

This is outstanding but still applies.

Regards
Adrian

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] perf intel-pt: Fix ip compression
  2016-08-10  8:32 ` Adrian Hunter
@ 2016-08-10 13:05   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 4+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-08-10 13:05 UTC (permalink / raw)
  To: Adrian Hunter; +Cc: Jiri Olsa, linux-kernel

Em Wed, Aug 10, 2016 at 11:32:21AM +0300, Adrian Hunter escreveu:
> On 20/07/16 12:00, Adrian Hunter wrote:
> > June 2015 Intel SDM introduced IP Compression types 4 and 6. Refer section
> > 36.4.2.2 Target IP (TIP) Packet - IP Compression.

> > Existing Intel PT packet decoder did not support type 4, and got type 6
> > wrong.  Because type 3 and type 4 have the same number of bytes, the packet
> > 'count' has been changed from being the number of ip bytes to being the
> > type code.  That allows the Intel PT decoder to correctly decide whether to
> > sign-extend or use the last ip.  However that also meant the code had to be
> > adjusted in a number of places.
> > 
> > Currently hardware is not using the new compression types, so this fix has
> > no effect on existing hardware.
> > 
> > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> > ---
> >  .../perf/util/intel-pt-decoder/intel-pt-decoder.c  | 44 +++++++++++-----------
> >  .../util/intel-pt-decoder/intel-pt-pkt-decoder.c   | 24 ++++++++----
> >  2 files changed, 40 insertions(+), 28 deletions(-)
 
> Hi
 
> This is outstanding but still applies.

Oops, will get that merged, sorry for the delay.

- Arnaldo

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [tip:perf/urgent] perf intel-pt: Fix ip compression
  2016-07-20  9:00 [PATCH] perf intel-pt: Fix ip compression Adrian Hunter
  2016-08-10  8:32 ` Adrian Hunter
@ 2016-08-16 18:13 ` tip-bot for Adrian Hunter
  1 sibling, 0 replies; 4+ messages in thread
From: tip-bot for Adrian Hunter @ 2016-08-16 18:13 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, adrian.hunter, jolsa, mingo, hpa, tglx

Commit-ID:  e1717e0485af4f47fc4da1e979ac817f9ad61b0f
Gitweb:     http://git.kernel.org/tip/e1717e0485af4f47fc4da1e979ac817f9ad61b0f
Author:     Adrian Hunter <adrian.hunter@intel.com>
AuthorDate: Wed, 20 Jul 2016 12:00:06 +0300
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 12 Aug 2016 14:39:48 -0300

perf intel-pt: Fix ip compression

The June 2015 Intel SDM introduced IP Compression types 4 and 6. Refer
to section 36.4.2.2 Target IP (TIP) Packet - IP Compression.

Existing Intel PT packet decoder did not support type 4, and got type 6
wrong.  Because type 3 and type 4 have the same number of bytes, the
packet 'count' has been changed from being the number of ip bytes to
being the type code.  That allows the Intel PT decoder to correctly
decide whether to sign-extend or use the last ip.  However that also
meant the code had to be adjusted in a number of places.

Currently hardware is not using the new compression types, so this fix
has no effect on existing hardware.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Link: http://lkml.kernel.org/r/1469005206-3049-1-git-send-email-adrian.hunter@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 .../perf/util/intel-pt-decoder/intel-pt-decoder.c  | 44 +++++++++++-----------
 .../util/intel-pt-decoder/intel-pt-pkt-decoder.c   | 24 ++++++++----
 2 files changed, 40 insertions(+), 28 deletions(-)

diff --git a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
index 9c8f15d..8ff6c6a 100644
--- a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
+++ b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
@@ -123,8 +123,6 @@ struct intel_pt_decoder {
 	bool have_calc_cyc_to_tsc;
 	int exec_mode;
 	unsigned int insn_bytes;
-	uint64_t sign_bit;
-	uint64_t sign_bits;
 	uint64_t period;
 	enum intel_pt_period_type period_type;
 	uint64_t tot_insn_cnt;
@@ -191,9 +189,6 @@ struct intel_pt_decoder *intel_pt_decoder_new(struct intel_pt_params *params)
 	decoder->data               = params->data;
 	decoder->return_compression = params->return_compression;
 
-	decoder->sign_bit           = (uint64_t)1 << 47;
-	decoder->sign_bits          = ~(((uint64_t)1 << 48) - 1);
-
 	decoder->period             = params->period;
 	decoder->period_type        = params->period_type;
 
@@ -362,21 +357,30 @@ int intel_pt__strerror(int code, char *buf, size_t buflen)
 	return 0;
 }
 
-static uint64_t intel_pt_calc_ip(struct intel_pt_decoder *decoder,
-				 const struct intel_pt_pkt *packet,
+static uint64_t intel_pt_calc_ip(const struct intel_pt_pkt *packet,
 				 uint64_t last_ip)
 {
 	uint64_t ip;
 
 	switch (packet->count) {
-	case 2:
+	case 1:
 		ip = (last_ip & (uint64_t)0xffffffffffff0000ULL) |
 		     packet->payload;
 		break;
-	case 4:
+	case 2:
 		ip = (last_ip & (uint64_t)0xffffffff00000000ULL) |
 		     packet->payload;
 		break;
+	case 3:
+		ip = packet->payload;
+		/* Sign-extend 6-byte ip */
+		if (ip & (uint64_t)0x800000000000ULL)
+			ip |= (uint64_t)0xffff000000000000ULL;
+		break;
+	case 4:
+		ip = (last_ip & (uint64_t)0xffff000000000000ULL) |
+		     packet->payload;
+		break;
 	case 6:
 		ip = packet->payload;
 		break;
@@ -384,16 +388,12 @@ static uint64_t intel_pt_calc_ip(struct intel_pt_decoder *decoder,
 		return 0;
 	}
 
-	if (ip & decoder->sign_bit)
-		return ip | decoder->sign_bits;
-
 	return ip;
 }
 
 static inline void intel_pt_set_last_ip(struct intel_pt_decoder *decoder)
 {
-	decoder->last_ip = intel_pt_calc_ip(decoder, &decoder->packet,
-					    decoder->last_ip);
+	decoder->last_ip = intel_pt_calc_ip(&decoder->packet, decoder->last_ip);
 }
 
 static inline void intel_pt_set_ip(struct intel_pt_decoder *decoder)
@@ -1657,6 +1657,12 @@ next:
 	}
 }
 
+static inline bool intel_pt_have_ip(struct intel_pt_decoder *decoder)
+{
+	return decoder->last_ip || decoder->packet.count == 0 ||
+	       decoder->packet.count == 3 || decoder->packet.count == 6;
+}
+
 /* Walk PSB+ packets to get in sync. */
 static int intel_pt_walk_psb(struct intel_pt_decoder *decoder)
 {
@@ -1677,8 +1683,7 @@ static int intel_pt_walk_psb(struct intel_pt_decoder *decoder)
 
 		case INTEL_PT_FUP:
 			decoder->pge = true;
-			if (decoder->last_ip || decoder->packet.count == 6 ||
-			    decoder->packet.count == 0) {
+			if (intel_pt_have_ip(decoder)) {
 				uint64_t current_ip = decoder->ip;
 
 				intel_pt_set_ip(decoder);
@@ -1767,8 +1772,7 @@ static int intel_pt_walk_to_ip(struct intel_pt_decoder *decoder)
 		case INTEL_PT_TIP_PGE:
 		case INTEL_PT_TIP:
 			decoder->pge = decoder->packet.type != INTEL_PT_TIP_PGD;
-			if (decoder->last_ip || decoder->packet.count == 6 ||
-			    decoder->packet.count == 0)
+			if (intel_pt_have_ip(decoder))
 				intel_pt_set_ip(decoder);
 			if (decoder->ip)
 				return 0;
@@ -1776,9 +1780,7 @@ static int intel_pt_walk_to_ip(struct intel_pt_decoder *decoder)
 
 		case INTEL_PT_FUP:
 			if (decoder->overflow) {
-				if (decoder->last_ip ||
-				    decoder->packet.count == 6 ||
-				    decoder->packet.count == 0)
+				if (intel_pt_have_ip(decoder))
 					intel_pt_set_ip(decoder);
 				if (decoder->ip)
 					return 0;
diff --git a/tools/perf/util/intel-pt-decoder/intel-pt-pkt-decoder.c b/tools/perf/util/intel-pt-decoder/intel-pt-pkt-decoder.c
index b1257c8..4f7b320 100644
--- a/tools/perf/util/intel-pt-decoder/intel-pt-pkt-decoder.c
+++ b/tools/perf/util/intel-pt-decoder/intel-pt-pkt-decoder.c
@@ -292,36 +292,46 @@ static int intel_pt_get_ip(enum intel_pt_pkt_type type, unsigned int byte,
 			   const unsigned char *buf, size_t len,
 			   struct intel_pt_pkt *packet)
 {
-	switch (byte >> 5) {
+	int ip_len;
+
+	packet->count = byte >> 5;
+
+	switch (packet->count) {
 	case 0:
-		packet->count = 0;
+		ip_len = 0;
 		break;
 	case 1:
 		if (len < 3)
 			return INTEL_PT_NEED_MORE_BYTES;
-		packet->count = 2;
+		ip_len = 2;
 		packet->payload = le16_to_cpu(*(uint16_t *)(buf + 1));
 		break;
 	case 2:
 		if (len < 5)
 			return INTEL_PT_NEED_MORE_BYTES;
-		packet->count = 4;
+		ip_len = 4;
 		packet->payload = le32_to_cpu(*(uint32_t *)(buf + 1));
 		break;
 	case 3:
-	case 6:
+	case 4:
 		if (len < 7)
 			return INTEL_PT_NEED_MORE_BYTES;
-		packet->count = 6;
+		ip_len = 6;
 		memcpy_le64(&packet->payload, buf + 1, 6);
 		break;
+	case 6:
+		if (len < 9)
+			return INTEL_PT_NEED_MORE_BYTES;
+		ip_len = 8;
+		packet->payload = le64_to_cpu(*(uint64_t *)(buf + 1));
+		break;
 	default:
 		return INTEL_PT_BAD_PACKET;
 	}
 
 	packet->type = type;
 
-	return packet->count + 1;
+	return ip_len + 1;
 }
 
 static int intel_pt_get_mode(const unsigned char *buf, size_t len,

^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2016-08-16 18:15 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-20  9:00 [PATCH] perf intel-pt: Fix ip compression Adrian Hunter
2016-08-10  8:32 ` Adrian Hunter
2016-08-10 13:05   ` Arnaldo Carvalho de Melo
2016-08-16 18:13 ` [tip:perf/urgent] " tip-bot for Adrian Hunter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.