[PATCH RESEND v1 05/13] perf arm-spe: Refactor packet header parsing

From: Leo Yan
Date: Mon Aug 17 2020 - 10:54:48 EST


The packet header parsing uses the hard coded values and it uses nested
if-else statements.

To improve the readability, this patch refactors the macros for packet
header format so it removes the hard coded values. Furthermore, based
on the new mask macros it reduces the nested if-else statements and
changes to use the flat conditions checking, this is directive and can
easily map to the descriptions in ARMv8-a architecture reference manual
(ARM DDI 0487E.a), chapter 'D10.1.5 Statistical Profiling Extension
protocol packet headers'.

Signed-off-by: Leo Yan <leo.yan@xxxxxxxxxx>
---
.../arm-spe-decoder/arm-spe-pkt-decoder.c | 92 +++++++++----------
.../arm-spe-decoder/arm-spe-pkt-decoder.h | 17 ++++
2 files changed, 58 insertions(+), 51 deletions(-)

diff --git a/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c b/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c
index 4f0aeb62e97b..5f4f900a9980 100644
--- a/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c
+++ b/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c
@@ -15,28 +15,6 @@
#define NS_FLAG BIT(63)
#define EL_FLAG (BIT(62) | BIT(61))

-#define SPE_HEADER0_PAD 0x0
-#define SPE_HEADER0_END 0x1
-#define SPE_HEADER0_ADDRESS 0x30 /* address packet (short) */
-#define SPE_HEADER0_ADDRESS_MASK 0x38
-#define SPE_HEADER0_COUNTER 0x18 /* counter packet (short) */
-#define SPE_HEADER0_COUNTER_MASK 0x38
-#define SPE_HEADER0_TIMESTAMP 0x71
-#define SPE_HEADER0_TIMESTAMP 0x71
-#define SPE_HEADER0_EVENTS 0x2
-#define SPE_HEADER0_EVENTS_MASK 0xf
-#define SPE_HEADER0_SOURCE 0x3
-#define SPE_HEADER0_SOURCE_MASK 0xf
-#define SPE_HEADER0_CONTEXT 0x24
-#define SPE_HEADER0_CONTEXT_MASK 0x3c
-#define SPE_HEADER0_OP_TYPE 0x8
-#define SPE_HEADER0_OP_TYPE_MASK 0x3c
-#define SPE_HEADER1_ALIGNMENT 0x0
-#define SPE_HEADER1_ADDRESS 0xb0 /* address packet (extended) */
-#define SPE_HEADER1_ADDRESS_MASK 0xf8
-#define SPE_HEADER1_COUNTER 0x98 /* counter packet (extended) */
-#define SPE_HEADER1_COUNTER_MASK 0xf8
-
#if __BYTE_ORDER == __BIG_ENDIAN
#define le16_to_cpu bswap_16
#define le32_to_cpu bswap_32
@@ -197,46 +175,58 @@ static int arm_spe_get_addr(const unsigned char *buf, size_t len,
static int arm_spe_do_get_packet(const unsigned char *buf, size_t len,
struct arm_spe_pkt *packet)
{
- unsigned int byte;
+ unsigned int hdr;
+ unsigned char ext_hdr = 0;

memset(packet, 0, sizeof(struct arm_spe_pkt));

if (!len)
return ARM_SPE_NEED_MORE_BYTES;

- byte = buf[0];
- if (byte == SPE_HEADER0_PAD)
+ hdr = buf[0];
+
+ if (hdr == SPE_HEADER0_PAD)
return arm_spe_get_pad(packet);
- else if (byte == SPE_HEADER0_END) /* no timestamp at end of record */
+
+ if (hdr == SPE_HEADER0_END) /* no timestamp at end of record */
return arm_spe_get_end(packet);
- else if (byte & 0xc0 /* 0y11xxxxxx */) {
- if (byte & 0x80) {
- if ((byte & SPE_HEADER0_ADDRESS_MASK) == SPE_HEADER0_ADDRESS)
- return arm_spe_get_addr(buf, len, 0, packet);
- if ((byte & SPE_HEADER0_COUNTER_MASK) == SPE_HEADER0_COUNTER)
- return arm_spe_get_counter(buf, len, 0, packet);
- } else
- if (byte == SPE_HEADER0_TIMESTAMP)
- return arm_spe_get_timestamp(buf, len, packet);
- else if ((byte & SPE_HEADER0_EVENTS_MASK) == SPE_HEADER0_EVENTS)
- return arm_spe_get_events(buf, len, packet);
- else if ((byte & SPE_HEADER0_SOURCE_MASK) == SPE_HEADER0_SOURCE)
- return arm_spe_get_data_source(buf, len, packet);
- else if ((byte & SPE_HEADER0_CONTEXT_MASK) == SPE_HEADER0_CONTEXT)
- return arm_spe_get_context(buf, len, packet);
- else if ((byte & SPE_HEADER0_OP_TYPE_MASK) == SPE_HEADER0_OP_TYPE)
- return arm_spe_get_op_type(buf, len, packet);
- } else if ((byte & 0xe0) == 0x20 /* 0y001xxxxx */) {
- /* 16-bit header */
- byte = buf[1];
- if (byte == SPE_HEADER1_ALIGNMENT)
+
+ if (hdr == SPE_HEADER0_TIMESTAMP)
+ return arm_spe_get_timestamp(buf, len, packet);
+
+ if ((hdr & SPE_HEADER0_MASK1) == SPE_HEADER0_EVENTS)
+ return arm_spe_get_events(buf, len, packet);
+
+ if ((hdr & SPE_HEADER0_MASK1) == SPE_HEADER0_SOURCE)
+ return arm_spe_get_data_source(buf, len, packet);
+
+ if ((hdr & SPE_HEADER0_MASK2) == SPE_HEADER0_CONTEXT)
+ return arm_spe_get_context(buf, len, packet);
+
+ if ((hdr & SPE_HEADER0_MASK2) == SPE_HEADER0_OPERATION)
+ return arm_spe_get_op_type(buf, len, packet);
+
+ if ((hdr & SPE_HEADER0_MASK3) == SPE_HEADER0_EXTENDED) {
+ /* 16-bit extended format header */
+ ext_hdr = 1;
+
+ hdr = buf[1];
+ if (hdr == SPE_HEADER1_ALIGNMENT)
return arm_spe_get_alignment(buf, len, packet);
- else if ((byte & SPE_HEADER1_ADDRESS_MASK) == SPE_HEADER1_ADDRESS)
- return arm_spe_get_addr(buf, len, 1, packet);
- else if ((byte & SPE_HEADER1_COUNTER_MASK) == SPE_HEADER1_COUNTER)
- return arm_spe_get_counter(buf, len, 1, packet);
}

+ /*
+ * The short format header's byte 0 or the extended format header's
+ * byte 1 has been assigned to 'hdr', which uses the same encoding for
+ * address packet and counter packet, so don't need to distinguish if
+ * it's short format or extended format and handle in once.
+ */
+ if ((hdr & SPE_HEADER0_MASK4) == SPE_HEADER0_ADDRESS)
+ return arm_spe_get_addr(buf, len, ext_hdr, packet);
+
+ if ((hdr & SPE_HEADER0_MASK4) == SPE_HEADER0_COUNTER)
+ return arm_spe_get_counter(buf, len, ext_hdr, packet);
+
return ARM_SPE_BAD_PACKET;
}

diff --git a/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.h b/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.h
index aca919159cad..9df5ebe02c5d 100644
--- a/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.h
+++ b/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.h
@@ -36,6 +36,23 @@ struct arm_spe_pkt {
uint64_t payload;
};

+/* Short header (HEADER0) and extended header (HEADER1) */
+#define SPE_HEADER0_PAD 0x0
+#define SPE_HEADER0_END 0x1
+#define SPE_HEADER0_TIMESTAMP 0x71
+#define SPE_HEADER0_MASK1 0xcf /* Mask for event & data source */
+#define SPE_HEADER0_EVENTS 0x42
+#define SPE_HEADER0_SOURCE 0x43
+#define SPE_HEADER0_MASK2 0xfc /* Mask for context & operation */
+#define SPE_HEADER0_CONTEXT 0x64
+#define SPE_HEADER0_OPERATION 0x48
+#define SPE_HEADER0_MASK3 0xe0 /* Mask for extended format */
+#define SPE_HEADER0_EXTENDED 0x20
+#define SPE_HEADER0_MASK4 0xf8 /* Mask for address & counter */
+#define SPE_HEADER0_ADDRESS 0xb0
+#define SPE_HEADER0_COUNTER 0x98
+#define SPE_HEADER1_ALIGNMENT 0x0
+
#define SPE_HEADER_SZ_SHIFT (4)
#define SPE_HEADER_SZ_MASK (0x30)

--
2.17.1