[PATCH 0/2] perf hisi-ptt: Fix TLP header parsing and naming

[PATCH 0/2] perf hisi-ptt: Fix TLP header parsing and naming

[Sizhe Liu]

This series fixes two issues in the hisi_ptt packet decoder that were
introduced by commit 5e91e57e6809 ("perf auxtrace arm64: Add support
for parsing HiSilicon PCIe Trace packet").

Patch 1 fixes spelling / abbreviation errors in symbol names and
macros.

Patch 2 fixes a bug:
The DW0 bit-field layout of the hisi_ptt_4dw union does not match
the actual little-endian memory ordering, so the extracted 
Format/Type/T9/T8/TH/SO/Length/Time fields are wrong.

Meanwhile, make all DW hex values printed in big-endian byte order
for readability.

Sizhe Liu (2):
  perf hisi-ptt: Fix spelling and abbreviation errors
  perf hisi-ptt: Fix PTT trace TLP header parsing

 Documentation/trace/hisi-ptt.rst              | 28 ++++-----
 .../hisi-ptt-decoder/hisi-ptt-pkt-decoder.c   | 62 +++++++++----------
 .../hisi-ptt-decoder/hisi-ptt-pkt-decoder.h   | 14 ++++-
 3 files changed, 58 insertions(+), 46 deletions(-)

-- 
2.33.0

[PATCH 1/2] perf hisi-ptt: Fix spelling and abbreviation errors

[Sizhe Liu]

Fix spelling and abbreviation errors in the PTT packet decoder:
- HISI_PTT_FIELD_LENTH -> HISI_PTT_FIELD_LENGTH
- hisi_ptt_8dw_kpt_desc -> hisi_ptt_8dw_pkt_desc
- hisi_ptt_4dw_kpt_desc -> hisi_ptt_4dw_pkt_desc

Cc: stable@vger.kernel.org
Fixes: 5e91e57e6809 ("perf auxtrace arm64: Add support for parsing HiSilicon PCIe Trace packet")
Signed-off-by: Sizhe Liu <liusizhe5@huawei.com>
---
 .../hisi-ptt-decoder/hisi-ptt-pkt-decoder.c   | 20 +++++++++----------
 .../hisi-ptt-decoder/hisi-ptt-pkt-decoder.h   |  2 +-
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
index a17c423a526d..c48b2ce7c4a3 100644
--- a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
+++ b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
@@ -94,26 +94,26 @@ static void hisi_ptt_print_pkt(const unsigned char *buf, int pos, const char *de
 
 	printf(".");
 	color_fprintf(stdout, color, "  %08x: ", pos);
-	for (i = 0; i < HISI_PTT_FIELD_LENTH; i++)
+	for (i = 0; i < HISI_PTT_FIELD_LENGTH; i++)
 		color_fprintf(stdout, color, "%02x ", buf[pos + i]);
 	for (i = 0; i < HISI_PTT_MAX_SPACE_LEN; i++)
 		color_fprintf(stdout, color, "   ");
 	color_fprintf(stdout, color, "  %s\n", desc);
 }
 
-static int hisi_ptt_8dw_kpt_desc(const unsigned char *buf, int pos)
+static int hisi_ptt_8dw_pkt_desc(const unsigned char *buf, int pos)
 {
 	int i;
 
 	for (i = 0; i < HISI_PTT_8DW_TYPE_MAX; i++) {
 		/* Do not show 8DW check field and reserved fields */
 		if (i == HISI_PTT_8DW_CHK_AND_RSV0 || i == HISI_PTT_8DW_RSV1) {
-			pos += HISI_PTT_FIELD_LENTH;
+			pos += HISI_PTT_FIELD_LENGTH;
 			continue;
 		}
 
 		hisi_ptt_print_pkt(buf, pos, hisi_ptt_8dw_pkt_field_name[i]);
-		pos += HISI_PTT_FIELD_LENTH;
+		pos += HISI_PTT_FIELD_LENGTH;
 	}
 
 	return hisi_ptt_pkt_size[HISI_PTT_8DW_PKT];
@@ -128,7 +128,7 @@ static void hisi_ptt_4dw_print_dw0(const unsigned char *buf, int pos)
 	dw0.value = *(uint32_t *)(buf + pos);
 	printf(".");
 	color_fprintf(stdout, color, "  %08x: ", pos);
-	for (i = 0; i < HISI_PTT_FIELD_LENTH; i++)
+	for (i = 0; i < HISI_PTT_FIELD_LENGTH; i++)
 		color_fprintf(stdout, color, "%02x ", buf[pos + i]);
 	for (i = 0; i < HISI_PTT_MAX_SPACE_LEN; i++)
 		color_fprintf(stdout, color, "   ");
@@ -140,16 +140,16 @@ static void hisi_ptt_4dw_print_dw0(const unsigned char *buf, int pos)
 		      dw0.len, "Time", dw0.time);
 }
 
-static int hisi_ptt_4dw_kpt_desc(const unsigned char *buf, int pos)
+static int hisi_ptt_4dw_pkt_desc(const unsigned char *buf, int pos)
 {
 	int i;
 
 	hisi_ptt_4dw_print_dw0(buf, pos);
-	pos += HISI_PTT_FIELD_LENTH;
+	pos += HISI_PTT_FIELD_LENGTH;
 
 	for (i = 0; i < HISI_PTT_4DW_TYPE_MAX; i++) {
 		hisi_ptt_print_pkt(buf, pos, hisi_ptt_4dw_pkt_field_name[i]);
-		pos += HISI_PTT_FIELD_LENTH;
+		pos += HISI_PTT_FIELD_LENGTH;
 	}
 
 	return hisi_ptt_pkt_size[HISI_PTT_4DW_PKT];
@@ -158,7 +158,7 @@ static int hisi_ptt_4dw_kpt_desc(const unsigned char *buf, int pos)
 int hisi_ptt_pkt_desc(const unsigned char *buf, int pos, enum hisi_ptt_pkt_type type)
 {
 	if (type == HISI_PTT_8DW_PKT)
-		return hisi_ptt_8dw_kpt_desc(buf, pos);
+		return hisi_ptt_8dw_pkt_desc(buf, pos);
 
-	return hisi_ptt_4dw_kpt_desc(buf, pos);
+	return hisi_ptt_4dw_pkt_desc(buf, pos);
 }
diff --git a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h
index e78f1b5bc836..6772b16b817b 100644
--- a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h
+++ b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h
@@ -13,7 +13,7 @@
 #define HISI_PTT_8DW_CHECK_MASK		GENMASK(31, 11)
 #define HISI_PTT_IS_8DW_PKT		GENMASK(31, 11)
 #define HISI_PTT_MAX_SPACE_LEN		10
-#define HISI_PTT_FIELD_LENTH		4
+#define HISI_PTT_FIELD_LENGTH		4
 
 enum hisi_ptt_pkt_type {
 	HISI_PTT_4DW_PKT,
-- 
2.33.0

[PATCH 2/2] perf hisi-ptt: Fix PTT trace TLP header parsing

[Sizhe Liu]

TLP Headers traced by HiSilicon PCIe tune and trace device (PTT) in
4DW format are shown in the document as below:
bits [31:30] [ 29:25 ][24][23][22][21][    20:11   ][    10:0    ]
     |-----|---------|---|---|---|---|-------------|-------------|
DW0  [ Fmt ][  Type  ][T9][T8][TH][SO][   Length   ][    Time    ]
DW1  [                     Header DW1                            ]
DW2  [                     Header DW2                            ]
DW3  [                     Header DW3                            ]

Problem:
The DW0 bit field layout of the hisi_ptt_4dw union does not match the
actual bit ordering in little-endian memory, causing incorrect field
decoding.

Test on Kunpeng 930 SOC, generating data flow with `iperf` commands:
- server side:
    iperf -s
- client side:
    iperf -c $ip_addr -t 30

Trace the TLP headers with hisi_ptt on server side at the same time:
  perf record -e hisi_ptt12_0/type=4,filter=0x05101,direction=2,format=0/ \
  --max-size 50M -o perf.data &
The trace aims to capture completion TLPs, learn more in the document:
  https://docs.kernel.org/trace/hisi-ptt.html

Decode perf.data with hisi_ptt decoder:
  perf report -D

The hisi_ptt decoder produces the following result:
[...perf headers and other information]
. ... HISI PTT data: size 8388608 bytes
.  00000000: 68 87 20 94                                 Format 3 Type 1a T9 0 T8 1 TH 1 SO 1 Length 10 Time 4a1
.  00000004: 40 00 00 00                                 Header DW1
.  00000008: 40 00 01 51                                 Header DW2
.  0000000c: 00 00 00 00                                 Header DW3
[...other hisi_ptt TLP headers]

According to PCIe r5.0 2.2.1, the Fmt & Type of Cpl/CplD is supposed to be
8b'00001010' / 8b'01001010'
However, the Format & Type decoder analyzing result is 8b'01111010'.
It does not match field encodings of any TLP messages.

Correct decoder result should be:
[...perf headers and other information]
. ... HISI PTT data: size 8388608 bytes
.  00000000: 94 20 87 68                                 Format 2 Type a T9 0 T8 0 TH 0 SO 1 Length 10 Time 768
.  00000004: 00 00 00 40                                 Header DW1
.  00000008: 51 01 00 40                                 Header DW2
.  0000000c: 00 00 00 00                                 Header DW3
[...other hisi_ptt TLP headers]

To solve the problem:
1. Drop the union and C bitfield struct, store the raw DW value in
a plain uint32_t, and extract the fields with FIELD_GET() against
GENMASK/BIT masks declared in the header so they can be reused by
other translation units. The masks are portable across endianness and
compilers.

2. Print all DW hex values in big-endian byte order for readability,
matching the bit field layout shown in the 4DW format diagram.

Cc: stable@vger.kernel.org
Fixes: 5e91e57e6809 ("perf auxtrace arm64: Add support for parsing HiSilicon PCIe Trace packet")
Signed-off-by: Sizhe Liu <liusizhe5@huawei.com>
---
 Documentation/trace/hisi-ptt.rst              | 28 +++++------
 .../hisi-ptt-decoder/hisi-ptt-pkt-decoder.c   | 46 +++++++++----------
 .../hisi-ptt-decoder/hisi-ptt-pkt-decoder.h   | 12 +++++
 3 files changed, 49 insertions(+), 37 deletions(-)

diff --git a/Documentation/trace/hisi-ptt.rst b/Documentation/trace/hisi-ptt.rst
index 6eef28ebb0c7..f6a2655f99e5 100644
--- a/Documentation/trace/hisi-ptt.rst
+++ b/Documentation/trace/hisi-ptt.rst
@@ -285,20 +285,20 @@ according to the format described previously (take 8DW as an example):
     [...perf headers and other information]
     . ... HISI PTT data: size 4194304 bytes
     .  00000000: 00 00 00 00                                 Prefix
-    .  00000004: 01 00 00 60                                 Header DW0
-    .  00000008: 0f 1e 00 01                                 Header DW1
-    .  0000000c: 04 00 00 00                                 Header DW2
-    .  00000010: 40 00 81 02                                 Header DW3
-    .  00000014: 33 c0 04 00                                 Time
+    .  00000004: 60 00 00 01                                 Header DW0
+    .  00000008: 01 00 1e 0f                                 Header DW1
+    .  0000000c: 00 00 00 04                                 Header DW2
+    .  00000010: 02 81 00 40                                 Header DW3
+    .  00000014: 00 04 c0 33                                 Time
     .  00000020: 00 00 00 00                                 Prefix
-    .  00000024: 01 00 00 60                                 Header DW0
-    .  00000028: 0f 1e 00 01                                 Header DW1
-    .  0000002c: 04 00 00 00                                 Header DW2
-    .  00000030: 40 00 81 02                                 Header DW3
-    .  00000034: 02 00 00 00                                 Time
+    .  00000024: 60 00 00 01                                 Header DW0
+    .  00000028: 01 00 1e 0f                                 Header DW1
+    .  0000002c: 00 00 00 04                                 Header DW2
+    .  00000030: 02 81 00 40                                 Header DW3
+    .  00000034: 00 00 00 02                                 Time
     .  00000040: 00 00 00 00                                 Prefix
-    .  00000044: 01 00 00 60                                 Header DW0
-    .  00000048: 0f 1e 00 01                                 Header DW1
-    .  0000004c: 04 00 00 00                                 Header DW2
-    .  00000050: 40 00 81 02                                 Header DW3
+    .  00000044: 60 00 00 01                                 Header DW0
+    .  00000048: 01 00 1e 0f                                 Header DW1
+    .  0000004c: 00 00 00 04                                 Header DW2
+    .  00000050: 02 81 00 40                                 Header DW3
     [...]
diff --git a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
index c48b2ce7c4a3..c1e169561087 100644
--- a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
+++ b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
@@ -10,6 +10,7 @@
 #include <endian.h>
 #include <byteswap.h>
 #include <linux/bitops.h>
+#include <linux/kernel.h>
 #include <stdarg.h>
 
 #include "../color.h"
@@ -73,29 +74,20 @@ static const char * const hisi_ptt_4dw_pkt_field_name[] = {
 	[HISI_PTT_4DW_HEAD3]	= "Header DW3",
 };
 
-union hisi_ptt_4dw {
-	struct {
-		uint32_t format : 2;
-		uint32_t type : 5;
-		uint32_t t9 : 1;
-		uint32_t t8 : 1;
-		uint32_t th : 1;
-		uint32_t so : 1;
-		uint32_t len : 10;
-		uint32_t time : 11;
-	};
-	uint32_t value;
-};
-
 static void hisi_ptt_print_pkt(const unsigned char *buf, int pos, const char *desc)
 {
 	const char *color = PERF_COLOR_BLUE;
+	uint32_t value;
+	uint8_t byte;
 	int i;
 
+	value = le32_to_cpu(*(__le32 *)(buf + pos));
 	printf(".");
 	color_fprintf(stdout, color, "  %08x: ", pos);
-	for (i = 0; i < HISI_PTT_FIELD_LENGTH; i++)
-		color_fprintf(stdout, color, "%02x ", buf[pos + i]);
+	for (i = 0; i < HISI_PTT_FIELD_LENGTH; i++) {
+		byte = (value >> (24 - i * 8)) & 0xFF;
+		color_fprintf(stdout, color, "%02x ", byte);
+	}
 	for (i = 0; i < HISI_PTT_MAX_SPACE_LEN; i++)
 		color_fprintf(stdout, color, "   ");
 	color_fprintf(stdout, color, "  %s\n", desc);
@@ -122,22 +114,30 @@ static int hisi_ptt_8dw_pkt_desc(const unsigned char *buf, int pos)
 static void hisi_ptt_4dw_print_dw0(const unsigned char *buf, int pos)
 {
 	const char *color = PERF_COLOR_BLUE;
-	union hisi_ptt_4dw dw0;
+	uint8_t byte;
+	uint32_t dw;
 	int i;
 
-	dw0.value = *(uint32_t *)(buf + pos);
+	dw = le32_to_cpu(*(__le32 *)(buf + pos));
 	printf(".");
 	color_fprintf(stdout, color, "  %08x: ", pos);
-	for (i = 0; i < HISI_PTT_FIELD_LENGTH; i++)
-		color_fprintf(stdout, color, "%02x ", buf[pos + i]);
+	for (i = 0; i < HISI_PTT_FIELD_LENGTH; i++) {
+		byte = (dw >> (24 - i * 8)) & 0xFF;
+		color_fprintf(stdout, color, "%02x ", byte);
+	}
 	for (i = 0; i < HISI_PTT_MAX_SPACE_LEN; i++)
 		color_fprintf(stdout, color, "   ");
 
 	color_fprintf(stdout, color,
 		      "  %s %x %s %x %s %x %s %x %s %x %s %x %s %x %s %x\n",
-		      "Format", dw0.format, "Type", dw0.type, "T9", dw0.t9,
-		      "T8", dw0.t8, "TH", dw0.th, "SO", dw0.so, "Length",
-		      dw0.len, "Time", dw0.time);
+		      "Format", FIELD_GET(HISI_PTT_HEAD0_4DW_FORMAT, dw),
+		      "Type", FIELD_GET(HISI_PTT_HEAD0_4DW_TYPE, dw),
+		      "T9", FIELD_GET(HISI_PTT_HEAD0_4DW_T9, dw),
+		      "T8", FIELD_GET(HISI_PTT_HEAD0_4DW_T8, dw),
+		      "TH", FIELD_GET(HISI_PTT_HEAD0_4DW_TH, dw),
+		      "SO", FIELD_GET(HISI_PTT_HEAD0_4DW_SO, dw),
+		      "Length", FIELD_GET(HISI_PTT_HEAD0_4DW_LEN, dw),
+		      "Time", FIELD_GET(HISI_PTT_HEAD0_4DW_TIME, dw));
 }
 
 static int hisi_ptt_4dw_pkt_desc(const unsigned char *buf, int pos)
diff --git a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h
index 6772b16b817b..3b349fcc24ae 100644
--- a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h
+++ b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h
@@ -9,12 +9,24 @@
 
 #include <stddef.h>
 #include <stdint.h>
+#include <linux/bits.h>
+#include <linux/bitfield.h>
 
 #define HISI_PTT_8DW_CHECK_MASK		GENMASK(31, 11)
 #define HISI_PTT_IS_8DW_PKT		GENMASK(31, 11)
 #define HISI_PTT_MAX_SPACE_LEN		10
 #define HISI_PTT_FIELD_LENGTH		4
 
+/* Header DW0 fields for 4DW format */
+#define HISI_PTT_HEAD0_4DW_TIME		GENMASK_U32(10, 0)
+#define HISI_PTT_HEAD0_4DW_LEN		GENMASK_U32(20, 11)
+#define HISI_PTT_HEAD0_4DW_SO		BIT_U32(21)
+#define HISI_PTT_HEAD0_4DW_TH		BIT_U32(22)
+#define HISI_PTT_HEAD0_4DW_T8		BIT_U32(23)
+#define HISI_PTT_HEAD0_4DW_T9		BIT_U32(24)
+#define HISI_PTT_HEAD0_4DW_TYPE		GENMASK_U32(29, 25)
+#define HISI_PTT_HEAD0_4DW_FORMAT	GENMASK_U32(31, 30)
+
 enum hisi_ptt_pkt_type {
 	HISI_PTT_4DW_PKT,
 	HISI_PTT_8DW_PKT,
-- 
2.33.0

Re: [PATCH 1/2] perf hisi-ptt: Fix spelling and abbreviation errors

[sashiko-bot]

> Fix spelling and abbreviation errors in the PTT packet decoder:
> - HISI_PTT_FIELD_LENTH -> HISI_PTT_FIELD_LENGTH
> - hisi_ptt_8dw_kpt_desc -> hisi_ptt_8dw_pkt_desc
> - hisi_ptt_4dw_kpt_desc -> hisi_ptt_4dw_pkt_desc
> 
> Cc: stable@vger.kernel.org
> Fixes: 5e91e57e6809 ("perf auxtrace arm64: Add support for parsing HiSilicon PCIe Trace packet")
> Signed-off-by: Sizhe Liu <liusizhe5@huawei.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729060220.624250-1-liusizhe5@huawei.com?part=1

Re: [PATCH 2/2] perf hisi-ptt: Fix PTT trace TLP header parsing

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [Medium] Incompatible pointer cast from `const unsigned char *` to `__le32 *` violates strict aliasing rules.

Pre-existing issues:
- [High] Incomplete cross-platform decoding fix: `hisi_ptt_check_packet_type` reads the packet header using a host-endian memory cast, breaking 8DW packet identification on Big-Endian hosts.
- [High] Heap out-of-bounds read when processing auxtrace events with a size less than 4 bytes, due to missing bounds checking before packet type evaluation.
- [High] Integer truncation of the 64-bit `event->auxtrace.size` leads to allocation failures and stream desynchronization.
--

--- Patch [2]: [PATCH 2/2] perf hisi-ptt: Fix PTT trace TLP header parsing ---
commit 637114d43d04dfaf1c2cef1c93751d90838d2eb7
Author: Sizhe Liu <liusizhe5@huawei.com>

perf hisi-ptt: Fix PTT trace TLP header parsing

The DW0 bit field layout of the hisi_ptt_4dw union does not match the
actual bit ordering in little-endian memory. This commit fixes the
decoding by replacing the bitfield union with FIELD_GET() operations
against GENMASK/BIT masks and printing DW values in big-endian order.

> diff --git a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
> index c48b2ce7c4a3d..c1e169561087c 100644
> --- a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
> +++ b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
[ ... ]
> @@ -73,29 +74,20 @@ static const char * const hisi_ptt_4dw_pkt_field_name[] = {
[ ... ]
>  static void hisi_ptt_print_pkt(const unsigned char *buf, int pos, const char *desc)
>  {
>  	const char *color = PERF_COLOR_BLUE;
> +	uint32_t value;
> +	uint8_t byte;
>  	int i;
>  
> +	value = le32_to_cpu(*(__le32 *)(buf + pos));

[Severity: Medium]
Does this incompatible pointer cast from const unsigned char * to __le32 *
violate strict aliasing rules? Would it be safer to use
get_unaligned_le32(buf + pos) instead?

>  	printf(".");
>  	color_fprintf(stdout, color, "  %08x: ", pos);
> -	for (i = 0; i < HISI_PTT_FIELD_LENGTH; i++)
> -		color_fprintf(stdout, color, "%02x ", buf[pos + i]);
> +	for (i = 0; i < HISI_PTT_FIELD_LENGTH; i++) {
> +		byte = (value >> (24 - i * 8)) & 0xFF;
> +		color_fprintf(stdout, color, "%02x ", byte);
> +	}
[ ... ]
> @@ -122,22 +114,30 @@ static int hisi_ptt_8dw_pkt_desc(const unsigned char *buf, int pos)
>  static void hisi_ptt_4dw_print_dw0(const unsigned char *buf, int pos)
>  {
>  	const char *color = PERF_COLOR_BLUE;
> -	union hisi_ptt_4dw dw0;
> +	uint8_t byte;
> +	uint32_t dw;
>  	int i;
>  
> -	dw0.value = *(uint32_t *)(buf + pos);
> +	dw = le32_to_cpu(*(__le32 *)(buf + pos));

[Severity: Medium]
Does this incompatible pointer cast from const unsigned char * to __le32 *
violate strict aliasing rules? Would it be safer to use
get_unaligned_le32(buf + pos) instead?

>  	printf(".");
>  	color_fprintf(stdout, color, "  %08x: ", pos);
> -	for (i = 0; i < HISI_PTT_FIELD_LENGTH; i++)
> -		color_fprintf(stdout, color, "%02x ", buf[pos + i]);
> +	for (i = 0; i < HISI_PTT_FIELD_LENGTH; i++) {
> +		byte = (dw >> (24 - i * 8)) & 0xFF;
> +		color_fprintf(stdout, color, "%02x ", byte);
> +	}
[ ... ]

[Severity: High]
This is a pre-existing issue, but does hisi_ptt_check_packet_type() break
8DW packet identification on big-endian hosts?

tools/perf/util/hisi-ptt.c:hisi_ptt_check_packet_type()
	uint32_t head = *(uint32_t *)buf;

	if ((HISI_PTT_8DW_CHECK_MASK & head) == HISI_PTT_IS_8DW_PKT)
		return HISI_PTT_8DW_PKT;

The function reads the first 32-bit word using a host-endian memory cast
without correcting for host endianness. When analyzing a little-endian trace
on a big-endian host, this would cause valid 8DW packets to fail the
bitwise mask check and be misclassified as 4DW packets.


[Severity: High]
This is a pre-existing issue, but can this sequence lead to a heap
out-of-bounds read when processing malformed auxtrace events?

If the user provides an event with an auxtrace.size between 0 and 3 bytes:

tools/perf/util/hisi-ptt.c:hisi_ptt_process_auxtrace_event()
	int size = event->auxtrace.size;
	void *data = malloc(size);
	...
	err = readn(fd, data, size);
	...
	if (dump_trace)
		hisi_ptt_dump_event(ptt, data, size);

The buffer is passed to hisi_ptt_dump(), which unconditionally dereferences
the first 4 bytes of the buffer inside hisi_ptt_check_packet_type() before
validating if the buffer is large enough:

tools/perf/util/hisi-ptt.c:hisi_ptt_dump()
	type = hisi_ptt_check_packet_type(buf);

tools/perf/util/hisi-ptt.c:hisi_ptt_check_packet_type()
	uint32_t head = *(uint32_t *)buf;

Could this cause a heap out-of-bounds read if the buffer is smaller than
4 bytes?


[Severity: High]
This is a pre-existing issue, but does integer truncation on
event->auxtrace.size cause allocation failures or stream desynchronization?

tools/perf/util/hisi-ptt.c:hisi_ptt_process_auxtrace_event()
	int size = event->auxtrace.size;
	void *data = malloc(size);
	...
	err = readn(fd, data, size);

The 64-bit user-controlled size is implicitly truncated to a 32-bit signed
integer. For large valid traces (e.g., > 2GB), the truncated size becomes
negative, causing malloc to fail. Furthermore, if a huge size is specified,
it truncates to a small positive integer, causing an undersized read in
readn(). Could this leave unread payload in the pipe and permanently
desynchronize the stream?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729060220.624250-1-liusizhe5@huawei.com?part=2

Re: [PATCH 2/2] perf hisi-ptt: Fix PTT trace TLP header parsing

[James Clark]

On 29/07/2026 7:02 am, Sizhe Liu wrote:
> TLP Headers traced by HiSilicon PCIe tune and trace device (PTT) in
> 4DW format are shown in the document as below:
> bits [31:30] [ 29:25 ][24][23][22][21][    20:11   ][    10:0    ]
>       |-----|---------|---|---|---|---|-------------|-------------|
> DW0  [ Fmt ][  Type  ][T9][T8][TH][SO][   Length   ][    Time    ]
> DW1  [                     Header DW1                            ]
> DW2  [                     Header DW2                            ]
> DW3  [                     Header DW3                            ]
> 
> Problem:
> The DW0 bit field layout of the hisi_ptt_4dw union does not match the
> actual bit ordering in little-endian memory, causing incorrect field
> decoding.
> 
> Test on Kunpeng 930 SOC, generating data flow with `iperf` commands:
> - server side:
>      iperf -s
> - client side:
>      iperf -c $ip_addr -t 30
> 
> Trace the TLP headers with hisi_ptt on server side at the same time:
>    perf record -e hisi_ptt12_0/type=4,filter=0x05101,direction=2,format=0/ \
>    --max-size 50M -o perf.data &
> The trace aims to capture completion TLPs, learn more in the document:
>    https://docs.kernel.org/trace/hisi-ptt.html
> 
> Decode perf.data with hisi_ptt decoder:
>    perf report -D
> 
> The hisi_ptt decoder produces the following result:
> [...perf headers and other information]
> . ... HISI PTT data: size 8388608 bytes
> .  00000000: 68 87 20 94                                 Format 3 Type 1a T9 0 T8 1 TH 1 SO 1 Length 10 Time 4a1
> .  00000004: 40 00 00 00                                 Header DW1
> .  00000008: 40 00 01 51                                 Header DW2
> .  0000000c: 00 00 00 00                                 Header DW3
> [...other hisi_ptt TLP headers]
> 
> According to PCIe r5.0 2.2.1, the Fmt & Type of Cpl/CplD is supposed to be
> 8b'00001010' / 8b'01001010'
> However, the Format & Type decoder analyzing result is 8b'01111010'.
> It does not match field encodings of any TLP messages.
> 
> Correct decoder result should be:
> [...perf headers and other information]
> . ... HISI PTT data: size 8388608 bytes
> .  00000000: 94 20 87 68                                 Format 2 Type a T9 0 T8 0 TH 0 SO 1 Length 10 Time 768
> .  00000004: 00 00 00 40                                 Header DW1
> .  00000008: 51 01 00 40                                 Header DW2
> .  0000000c: 00 00 00 00                                 Header DW3
> [...other hisi_ptt TLP headers]
> 
> To solve the problem:
> 1. Drop the union and C bitfield struct, store the raw DW value in
> a plain uint32_t, and extract the fields with FIELD_GET() against
> GENMASK/BIT masks declared in the header so they can be reused by
> other translation units. The masks are portable across endianness and
> compilers.
> 
> 2. Print all DW hex values in big-endian byte order for readability,
> matching the bit field layout shown in the 4DW format diagram.
> 
> Cc: stable@vger.kernel.org
> Fixes: 5e91e57e6809 ("perf auxtrace arm64: Add support for parsing HiSilicon PCIe Trace packet")
> Signed-off-by: Sizhe Liu <liusizhe5@huawei.com>
> ---
>   Documentation/trace/hisi-ptt.rst              | 28 +++++------
>   .../hisi-ptt-decoder/hisi-ptt-pkt-decoder.c   | 46 +++++++++----------
>   .../hisi-ptt-decoder/hisi-ptt-pkt-decoder.h   | 12 +++++
>   3 files changed, 49 insertions(+), 37 deletions(-)
> 
> diff --git a/Documentation/trace/hisi-ptt.rst b/Documentation/trace/hisi-ptt.rst
> index 6eef28ebb0c7..f6a2655f99e5 100644
> --- a/Documentation/trace/hisi-ptt.rst
> +++ b/Documentation/trace/hisi-ptt.rst
> @@ -285,20 +285,20 @@ according to the format described previously (take 8DW as an example):
>       [...perf headers and other information]
>       . ... HISI PTT data: size 4194304 bytes
>       .  00000000: 00 00 00 00                                 Prefix
> -    .  00000004: 01 00 00 60                                 Header DW0
> -    .  00000008: 0f 1e 00 01                                 Header DW1
> -    .  0000000c: 04 00 00 00                                 Header DW2
> -    .  00000010: 40 00 81 02                                 Header DW3
> -    .  00000014: 33 c0 04 00                                 Time
> +    .  00000004: 60 00 00 01                                 Header DW0
> +    .  00000008: 01 00 1e 0f                                 Header DW1
> +    .  0000000c: 00 00 00 04                                 Header DW2
> +    .  00000010: 02 81 00 40                                 Header DW3
> +    .  00000014: 00 04 c0 33                                 Time
>       .  00000020: 00 00 00 00                                 Prefix
> -    .  00000024: 01 00 00 60                                 Header DW0
> -    .  00000028: 0f 1e 00 01                                 Header DW1
> -    .  0000002c: 04 00 00 00                                 Header DW2
> -    .  00000030: 40 00 81 02                                 Header DW3
> -    .  00000034: 02 00 00 00                                 Time
> +    .  00000024: 60 00 00 01                                 Header DW0
> +    .  00000028: 01 00 1e 0f                                 Header DW1
> +    .  0000002c: 00 00 00 04                                 Header DW2
> +    .  00000030: 02 81 00 40                                 Header DW3
> +    .  00000034: 00 00 00 02                                 Time
>       .  00000040: 00 00 00 00                                 Prefix
> -    .  00000044: 01 00 00 60                                 Header DW0
> -    .  00000048: 0f 1e 00 01                                 Header DW1
> -    .  0000004c: 04 00 00 00                                 Header DW2
> -    .  00000050: 40 00 81 02                                 Header DW3
> +    .  00000044: 60 00 00 01                                 Header DW0
> +    .  00000048: 01 00 1e 0f                                 Header DW1
> +    .  0000004c: 00 00 00 04                                 Header DW2
> +    .  00000050: 02 81 00 40                                 Header DW3
>       [...]
> diff --git a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
> index c48b2ce7c4a3..c1e169561087 100644
> --- a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
> +++ b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
> @@ -10,6 +10,7 @@
>   #include <endian.h>
>   #include <byteswap.h>
>   #include <linux/bitops.h>
> +#include <linux/kernel.h>
>   #include <stdarg.h>
>   
>   #include "../color.h"
> @@ -73,29 +74,20 @@ static const char * const hisi_ptt_4dw_pkt_field_name[] = {
>   	[HISI_PTT_4DW_HEAD3]	= "Header DW3",
>   };
>   
> -union hisi_ptt_4dw {
> -	struct {
> -		uint32_t format : 2;
> -		uint32_t type : 5;
> -		uint32_t t9 : 1;
> -		uint32_t t8 : 1;
> -		uint32_t th : 1;
> -		uint32_t so : 1;
> -		uint32_t len : 10;
> -		uint32_t time : 11;
> -	};
> -	uint32_t value;
> -};
> -
>   static void hisi_ptt_print_pkt(const unsigned char *buf, int pos, const char *desc)
>   {
>   	const char *color = PERF_COLOR_BLUE;
> +	uint32_t value;
> +	uint8_t byte;
>   	int i;
>   
> +	value = le32_to_cpu(*(__le32 *)(buf + pos));
>   	printf(".");
>   	color_fprintf(stdout, color, "  %08x: ", pos);
> -	for (i = 0; i < HISI_PTT_FIELD_LENGTH; i++)
> -		color_fprintf(stdout, color, "%02x ", buf[pos + i]);
> +	for (i = 0; i < HISI_PTT_FIELD_LENGTH; i++) {
> +		byte = (value >> (24 - i * 8)) & 0xFF;
> +		color_fprintf(stdout, color, "%02x ", byte);
> +	}
>   	for (i = 0; i < HISI_PTT_MAX_SPACE_LEN; i++)
>   		color_fprintf(stdout, color, "   ");
>   	color_fprintf(stdout, color, "  %s\n", desc);
> @@ -122,22 +114,30 @@ static int hisi_ptt_8dw_pkt_desc(const unsigned char *buf, int pos)
>   static void hisi_ptt_4dw_print_dw0(const unsigned char *buf, int pos)
>   {
>   	const char *color = PERF_COLOR_BLUE;
> -	union hisi_ptt_4dw dw0;
> +	uint8_t byte;
> +	uint32_t dw;
>   	int i;
>   
> -	dw0.value = *(uint32_t *)(buf + pos);
> +	dw = le32_to_cpu(*(__le32 *)(buf + pos));

Sashiko and the comment in generic.h say to use get_unaligned_le32() for 
unaligned data. It looks like this always is aligned but it's not 
enforced by the function, so to avoid anyone wondering if it's correct I 
would just use get_unaligned_le32().

With that:

Reviewed-by: James Clark <james.clark@linaro.org>

Sashiko also says there is another endianess issue in 
hisi_ptt_check_packet_type(). Up to you whether you want to fix it or 
not: 
https://sashiko.dev/#/patchset/20260729060220.624250-1-liusizhe5%40huawei.com

>   	printf(".");
>   	color_fprintf(stdout, color, "  %08x: ", pos);
> -	for (i = 0; i < HISI_PTT_FIELD_LENGTH; i++)
> -		color_fprintf(stdout, color, "%02x ", buf[pos + i]);
> +	for (i = 0; i < HISI_PTT_FIELD_LENGTH; i++) {
> +		byte = (dw >> (24 - i * 8)) & 0xFF;
> +		color_fprintf(stdout, color, "%02x ", byte);
> +	}
>   	for (i = 0; i < HISI_PTT_MAX_SPACE_LEN; i++)
>   		color_fprintf(stdout, color, "   ");
>   
>   	color_fprintf(stdout, color,
>   		      "  %s %x %s %x %s %x %s %x %s %x %s %x %s %x %s %x\n",
> -		      "Format", dw0.format, "Type", dw0.type, "T9", dw0.t9,
> -		      "T8", dw0.t8, "TH", dw0.th, "SO", dw0.so, "Length",
> -		      dw0.len, "Time", dw0.time);
> +		      "Format", FIELD_GET(HISI_PTT_HEAD0_4DW_FORMAT, dw),
> +		      "Type", FIELD_GET(HISI_PTT_HEAD0_4DW_TYPE, dw),
> +		      "T9", FIELD_GET(HISI_PTT_HEAD0_4DW_T9, dw),
> +		      "T8", FIELD_GET(HISI_PTT_HEAD0_4DW_T8, dw),
> +		      "TH", FIELD_GET(HISI_PTT_HEAD0_4DW_TH, dw),
> +		      "SO", FIELD_GET(HISI_PTT_HEAD0_4DW_SO, dw),
> +		      "Length", FIELD_GET(HISI_PTT_HEAD0_4DW_LEN, dw),
> +		      "Time", FIELD_GET(HISI_PTT_HEAD0_4DW_TIME, dw));
>   }
>   
>   static int hisi_ptt_4dw_pkt_desc(const unsigned char *buf, int pos)
> diff --git a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h
> index 6772b16b817b..3b349fcc24ae 100644
> --- a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h
> +++ b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h
> @@ -9,12 +9,24 @@
>   
>   #include <stddef.h>
>   #include <stdint.h>
> +#include <linux/bits.h>
> +#include <linux/bitfield.h>
>   
>   #define HISI_PTT_8DW_CHECK_MASK		GENMASK(31, 11)
>   #define HISI_PTT_IS_8DW_PKT		GENMASK(31, 11)
>   #define HISI_PTT_MAX_SPACE_LEN		10
>   #define HISI_PTT_FIELD_LENGTH		4
>   
> +/* Header DW0 fields for 4DW format */
> +#define HISI_PTT_HEAD0_4DW_TIME		GENMASK_U32(10, 0)
> +#define HISI_PTT_HEAD0_4DW_LEN		GENMASK_U32(20, 11)
> +#define HISI_PTT_HEAD0_4DW_SO		BIT_U32(21)
> +#define HISI_PTT_HEAD0_4DW_TH		BIT_U32(22)
> +#define HISI_PTT_HEAD0_4DW_T8		BIT_U32(23)
> +#define HISI_PTT_HEAD0_4DW_T9		BIT_U32(24)
> +#define HISI_PTT_HEAD0_4DW_TYPE		GENMASK_U32(29, 25)
> +#define HISI_PTT_HEAD0_4DW_FORMAT	GENMASK_U32(31, 30)
> +
>   enum hisi_ptt_pkt_type {
>   	HISI_PTT_4DW_PKT,
>   	HISI_PTT_8DW_PKT,

Re: [PATCH 1/2] perf hisi-ptt: Fix spelling and abbreviation errors

[James Clark]

On 29/07/2026 7:02 am, Sizhe Liu wrote:
> Fix spelling and abbreviation errors in the PTT packet decoder:
> - HISI_PTT_FIELD_LENTH -> HISI_PTT_FIELD_LENGTH
> - hisi_ptt_8dw_kpt_desc -> hisi_ptt_8dw_pkt_desc
> - hisi_ptt_4dw_kpt_desc -> hisi_ptt_4dw_pkt_desc
> 
> Cc: stable@vger.kernel.org
> Fixes: 5e91e57e6809 ("perf auxtrace arm64: Add support for parsing HiSilicon PCIe Trace packet")

Probably doesn't need a fixes tag, but:

Reviewed-by: James Clark <james.clark@linaro.org>

> Signed-off-by: Sizhe Liu <liusizhe5@huawei.com>
> ---
>   .../hisi-ptt-decoder/hisi-ptt-pkt-decoder.c   | 20 +++++++++----------
>   .../hisi-ptt-decoder/hisi-ptt-pkt-decoder.h   |  2 +-
>   2 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
> index a17c423a526d..c48b2ce7c4a3 100644
> --- a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
> +++ b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
> @@ -94,26 +94,26 @@ static void hisi_ptt_print_pkt(const unsigned char *buf, int pos, const char *de
>   
>   	printf(".");
>   	color_fprintf(stdout, color, "  %08x: ", pos);
> -	for (i = 0; i < HISI_PTT_FIELD_LENTH; i++)
> +	for (i = 0; i < HISI_PTT_FIELD_LENGTH; i++)
>   		color_fprintf(stdout, color, "%02x ", buf[pos + i]);
>   	for (i = 0; i < HISI_PTT_MAX_SPACE_LEN; i++)
>   		color_fprintf(stdout, color, "   ");
>   	color_fprintf(stdout, color, "  %s\n", desc);
>   }
>   
> -static int hisi_ptt_8dw_kpt_desc(const unsigned char *buf, int pos)
> +static int hisi_ptt_8dw_pkt_desc(const unsigned char *buf, int pos)
>   {
>   	int i;
>   
>   	for (i = 0; i < HISI_PTT_8DW_TYPE_MAX; i++) {
>   		/* Do not show 8DW check field and reserved fields */
>   		if (i == HISI_PTT_8DW_CHK_AND_RSV0 || i == HISI_PTT_8DW_RSV1) {
> -			pos += HISI_PTT_FIELD_LENTH;
> +			pos += HISI_PTT_FIELD_LENGTH;
>   			continue;
>   		}
>   
>   		hisi_ptt_print_pkt(buf, pos, hisi_ptt_8dw_pkt_field_name[i]);
> -		pos += HISI_PTT_FIELD_LENTH;
> +		pos += HISI_PTT_FIELD_LENGTH;
>   	}
>   
>   	return hisi_ptt_pkt_size[HISI_PTT_8DW_PKT];
> @@ -128,7 +128,7 @@ static void hisi_ptt_4dw_print_dw0(const unsigned char *buf, int pos)
>   	dw0.value = *(uint32_t *)(buf + pos);
>   	printf(".");
>   	color_fprintf(stdout, color, "  %08x: ", pos);
> -	for (i = 0; i < HISI_PTT_FIELD_LENTH; i++)
> +	for (i = 0; i < HISI_PTT_FIELD_LENGTH; i++)
>   		color_fprintf(stdout, color, "%02x ", buf[pos + i]);
>   	for (i = 0; i < HISI_PTT_MAX_SPACE_LEN; i++)
>   		color_fprintf(stdout, color, "   ");
> @@ -140,16 +140,16 @@ static void hisi_ptt_4dw_print_dw0(const unsigned char *buf, int pos)
>   		      dw0.len, "Time", dw0.time);
>   }
>   
> -static int hisi_ptt_4dw_kpt_desc(const unsigned char *buf, int pos)
> +static int hisi_ptt_4dw_pkt_desc(const unsigned char *buf, int pos)
>   {
>   	int i;
>   
>   	hisi_ptt_4dw_print_dw0(buf, pos);
> -	pos += HISI_PTT_FIELD_LENTH;
> +	pos += HISI_PTT_FIELD_LENGTH;
>   
>   	for (i = 0; i < HISI_PTT_4DW_TYPE_MAX; i++) {
>   		hisi_ptt_print_pkt(buf, pos, hisi_ptt_4dw_pkt_field_name[i]);
> -		pos += HISI_PTT_FIELD_LENTH;
> +		pos += HISI_PTT_FIELD_LENGTH;
>   	}
>   
>   	return hisi_ptt_pkt_size[HISI_PTT_4DW_PKT];
> @@ -158,7 +158,7 @@ static int hisi_ptt_4dw_kpt_desc(const unsigned char *buf, int pos)
>   int hisi_ptt_pkt_desc(const unsigned char *buf, int pos, enum hisi_ptt_pkt_type type)
>   {
>   	if (type == HISI_PTT_8DW_PKT)
> -		return hisi_ptt_8dw_kpt_desc(buf, pos);
> +		return hisi_ptt_8dw_pkt_desc(buf, pos);
>   
> -	return hisi_ptt_4dw_kpt_desc(buf, pos);
> +	return hisi_ptt_4dw_pkt_desc(buf, pos);
>   }
> diff --git a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h
> index e78f1b5bc836..6772b16b817b 100644
> --- a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h
> +++ b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h
> @@ -13,7 +13,7 @@
>   #define HISI_PTT_8DW_CHECK_MASK		GENMASK(31, 11)
>   #define HISI_PTT_IS_8DW_PKT		GENMASK(31, 11)
>   #define HISI_PTT_MAX_SPACE_LEN		10
> -#define HISI_PTT_FIELD_LENTH		4
> +#define HISI_PTT_FIELD_LENGTH		4
>   
>   enum hisi_ptt_pkt_type {
>   	HISI_PTT_4DW_PKT,

Re: [PATCH 2/2] perf hisi-ptt: Fix PTT trace TLP header parsing

[Sizhe Liu]

On 2026/7/29 18:16, James Clark wrote:
>
>
> On 29/07/2026 7:02 am, Sizhe Liu wrote:
>> TLP Headers traced by HiSilicon PCIe tune and trace device (PTT) in
>> 4DW format are shown in the document as below:
>> bits [31:30] [ 29:25 ][24][23][22][21][    20:11   ][    10:0 ]
>> |-----|---------|---|---|---|---|-------------|-------------|
>> DW0  [ Fmt ][  Type  ][T9][T8][TH][SO][   Length   ][    Time ]
>> DW1  [                     Header DW1 ]
>> DW2  [                     Header DW2 ]
>> DW3  [                     Header DW3 ]
>>
>> Problem:
>> The DW0 bit field layout of the hisi_ptt_4dw union does not match the
>> actual bit ordering in little-endian memory, causing incorrect field
>> decoding.
>>
>> Test on Kunpeng 930 SOC, generating data flow with `iperf` commands:
>> - server side:
>>      iperf -s
>> - client side:
>>      iperf -c $ip_addr -t 30
>>
>> Trace the TLP headers with hisi_ptt on server side at the same time:
>>    perf record -e 
>> hisi_ptt12_0/type=4,filter=0x05101,direction=2,format=0/ \
>>    --max-size 50M -o perf.data &
>> The trace aims to capture completion TLPs, learn more in the document:
>>    https://docs.kernel.org/trace/hisi-ptt.html
>>
>> Decode perf.data with hisi_ptt decoder:
>>    perf report -D
>>
>> The hisi_ptt decoder produces the following result:
>> [...perf headers and other information]
>> . ... HISI PTT data: size 8388608 bytes
>> .  00000000: 68 87 20 94                                 Format 3 
>> Type 1a T9 0 T8 1 TH 1 SO 1 Length 10 Time 4a1
>> .  00000004: 40 00 00 00                                 Header DW1
>> .  00000008: 40 00 01 51                                 Header DW2
>> .  0000000c: 00 00 00 00                                 Header DW3
>> [...other hisi_ptt TLP headers]
>>
>> According to PCIe r5.0 2.2.1, the Fmt & Type of Cpl/CplD is supposed 
>> to be
>> 8b'00001010' / 8b'01001010'
>> However, the Format & Type decoder analyzing result is 8b'01111010'.
>> It does not match field encodings of any TLP messages.
>>
>> Correct decoder result should be:
>> [...perf headers and other information]
>> . ... HISI PTT data: size 8388608 bytes
>> .  00000000: 94 20 87 68                                 Format 2 
>> Type a T9 0 T8 0 TH 0 SO 1 Length 10 Time 768
>> .  00000004: 00 00 00 40                                 Header DW1
>> .  00000008: 51 01 00 40                                 Header DW2
>> .  0000000c: 00 00 00 00                                 Header DW3
>> [...other hisi_ptt TLP headers]
>>
>> To solve the problem:
>> 1. Drop the union and C bitfield struct, store the raw DW value in
>> a plain uint32_t, and extract the fields with FIELD_GET() against
>> GENMASK/BIT masks declared in the header so they can be reused by
>> other translation units. The masks are portable across endianness and
>> compilers.
>>
>> 2. Print all DW hex values in big-endian byte order for readability,
>> matching the bit field layout shown in the 4DW format diagram.
>>
>> Cc: stable@vger.kernel.org
>> Fixes: 5e91e57e6809 ("perf auxtrace arm64: Add support for parsing 
>> HiSilicon PCIe Trace packet")
>> Signed-off-by: Sizhe Liu <liusizhe5@huawei.com>
>> ---
>>   Documentation/trace/hisi-ptt.rst              | 28 +++++------
>>   .../hisi-ptt-decoder/hisi-ptt-pkt-decoder.c   | 46 +++++++++----------
>>   .../hisi-ptt-decoder/hisi-ptt-pkt-decoder.h   | 12 +++++
>>   3 files changed, 49 insertions(+), 37 deletions(-)
>>
>> diff --git a/Documentation/trace/hisi-ptt.rst 
>> b/Documentation/trace/hisi-ptt.rst
>> index 6eef28ebb0c7..f6a2655f99e5 100644
>> --- a/Documentation/trace/hisi-ptt.rst
>> +++ b/Documentation/trace/hisi-ptt.rst
>> @@ -285,20 +285,20 @@ according to the format described previously 
>> (take 8DW as an example):
>>       [...perf headers and other information]
>>       . ... HISI PTT data: size 4194304 bytes
>>       .  00000000: 00 00 00 00 Prefix
>> -    .  00000004: 01 00 00 60 Header DW0
>> -    .  00000008: 0f 1e 00 01 Header DW1
>> -    .  0000000c: 04 00 00 00 Header DW2
>> -    .  00000010: 40 00 81 02 Header DW3
>> -    .  00000014: 33 c0 04 00 Time
>> +    .  00000004: 60 00 00 01 Header DW0
>> +    .  00000008: 01 00 1e 0f Header DW1
>> +    .  0000000c: 00 00 00 04 Header DW2
>> +    .  00000010: 02 81 00 40 Header DW3
>> +    .  00000014: 00 04 c0 33 Time
>>       .  00000020: 00 00 00 00 Prefix
>> -    .  00000024: 01 00 00 60 Header DW0
>> -    .  00000028: 0f 1e 00 01 Header DW1
>> -    .  0000002c: 04 00 00 00 Header DW2
>> -    .  00000030: 40 00 81 02 Header DW3
>> -    .  00000034: 02 00 00 00 Time
>> +    .  00000024: 60 00 00 01 Header DW0
>> +    .  00000028: 01 00 1e 0f Header DW1
>> +    .  0000002c: 00 00 00 04 Header DW2
>> +    .  00000030: 02 81 00 40 Header DW3
>> +    .  00000034: 00 00 00 02 Time
>>       .  00000040: 00 00 00 00 Prefix
>> -    .  00000044: 01 00 00 60 Header DW0
>> -    .  00000048: 0f 1e 00 01 Header DW1
>> -    .  0000004c: 04 00 00 00 Header DW2
>> -    .  00000050: 40 00 81 02 Header DW3
>> +    .  00000044: 60 00 00 01 Header DW0
>> +    .  00000048: 01 00 1e 0f Header DW1
>> +    .  0000004c: 00 00 00 04 Header DW2
>> +    .  00000050: 02 81 00 40 Header DW3
>>       [...]
>> diff --git a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c 
>> b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
>> index c48b2ce7c4a3..c1e169561087 100644
>> --- a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
>> +++ b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
>> @@ -10,6 +10,7 @@
>>   #include <endian.h>
>>   #include <byteswap.h>
>>   #include <linux/bitops.h>
>> +#include <linux/kernel.h>
>>   #include <stdarg.h>
>>     #include "../color.h"
>> @@ -73,29 +74,20 @@ static const char * const 
>> hisi_ptt_4dw_pkt_field_name[] = {
>>       [HISI_PTT_4DW_HEAD3]    = "Header DW3",
>>   };
>>   -union hisi_ptt_4dw {
>> -    struct {
>> -        uint32_t format : 2;
>> -        uint32_t type : 5;
>> -        uint32_t t9 : 1;
>> -        uint32_t t8 : 1;
>> -        uint32_t th : 1;
>> -        uint32_t so : 1;
>> -        uint32_t len : 10;
>> -        uint32_t time : 11;
>> -    };
>> -    uint32_t value;
>> -};
>> -
>>   static void hisi_ptt_print_pkt(const unsigned char *buf, int pos, 
>> const char *desc)
>>   {
>>       const char *color = PERF_COLOR_BLUE;
>> +    uint32_t value;
>> +    uint8_t byte;
>>       int i;
>>   +    value = le32_to_cpu(*(__le32 *)(buf + pos));
>>       printf(".");
>>       color_fprintf(stdout, color, "  %08x: ", pos);
>> -    for (i = 0; i < HISI_PTT_FIELD_LENGTH; i++)
>> -        color_fprintf(stdout, color, "%02x ", buf[pos + i]);
>> +    for (i = 0; i < HISI_PTT_FIELD_LENGTH; i++) {
>> +        byte = (value >> (24 - i * 8)) & 0xFF;
>> +        color_fprintf(stdout, color, "%02x ", byte);
>> +    }
>>       for (i = 0; i < HISI_PTT_MAX_SPACE_LEN; i++)
>>           color_fprintf(stdout, color, "   ");
>>       color_fprintf(stdout, color, "  %s\n", desc);
>> @@ -122,22 +114,30 @@ static int hisi_ptt_8dw_pkt_desc(const unsigned 
>> char *buf, int pos)
>>   static void hisi_ptt_4dw_print_dw0(const unsigned char *buf, int pos)
>>   {
>>       const char *color = PERF_COLOR_BLUE;
>> -    union hisi_ptt_4dw dw0;
>> +    uint8_t byte;
>> +    uint32_t dw;
>>       int i;
>>   -    dw0.value = *(uint32_t *)(buf + pos);
>> +    dw = le32_to_cpu(*(__le32 *)(buf + pos));
>
> Sashiko and the comment in generic.h say to use get_unaligned_le32() 
> for unaligned data. It looks like this always is aligned but it's not 
> enforced by the function, so to avoid anyone wondering if it's correct 
> I would just use get_unaligned_le32().
>
> With that:
>
> Reviewed-by: James Clark <james.clark@linaro.org>
>
> Sashiko also says there is another endianess issue in 
> hisi_ptt_check_packet_type(). Up to you whether you want to fix it or 
> not: 
> https://sashiko.dev/#/patchset/20260729060220.624250-1-liusizhe5%40huawei.com
>
Thanks for the review and the Reviewed-by tag.
I agree with using get_unaligned_le32(), to make it more robust. I'am 
updating it in the next version along with other endianess issuses.

Regards,
Sizhe
>>       printf(".");
>>       color_fprintf(stdout, color, "  %08x: ", pos);
>> -    for (i = 0; i < HISI_PTT_FIELD_LENGTH; i++)
>> -        color_fprintf(stdout, color, "%02x ", buf[pos + i]);
>> +    for (i = 0; i < HISI_PTT_FIELD_LENGTH; i++) {
>> +        byte = (dw >> (24 - i * 8)) & 0xFF;
>> +        color_fprintf(stdout, color, "%02x ", byte);
>> +    }
>>       for (i = 0; i < HISI_PTT_MAX_SPACE_LEN; i++)
>>           color_fprintf(stdout, color, "   ");
>>         color_fprintf(stdout, color,
>>                 "  %s %x %s %x %s %x %s %x %s %x %s %x %s %x %s %x\n",
>> -              "Format", dw0.format, "Type", dw0.type, "T9", dw0.t9,
>> -              "T8", dw0.t8, "TH", dw0.th, "SO", dw0.so, "Length",
>> -              dw0.len, "Time", dw0.time);
>> +              "Format", FIELD_GET(HISI_PTT_HEAD0_4DW_FORMAT, dw),
>> +              "Type", FIELD_GET(HISI_PTT_HEAD0_4DW_TYPE, dw),
>> +              "T9", FIELD_GET(HISI_PTT_HEAD0_4DW_T9, dw),
>> +              "T8", FIELD_GET(HISI_PTT_HEAD0_4DW_T8, dw),
>> +              "TH", FIELD_GET(HISI_PTT_HEAD0_4DW_TH, dw),
>> +              "SO", FIELD_GET(HISI_PTT_HEAD0_4DW_SO, dw),
>> +              "Length", FIELD_GET(HISI_PTT_HEAD0_4DW_LEN, dw),
>> +              "Time", FIELD_GET(HISI_PTT_HEAD0_4DW_TIME, dw));
>>   }
>>     static int hisi_ptt_4dw_pkt_desc(const unsigned char *buf, int pos)
>> diff --git a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h 
>> b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h
>> index 6772b16b817b..3b349fcc24ae 100644
>> --- a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h
>> +++ b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.h
>> @@ -9,12 +9,24 @@
>>     #include <stddef.h>
>>   #include <stdint.h>
>> +#include <linux/bits.h>
>> +#include <linux/bitfield.h>
>>     #define HISI_PTT_8DW_CHECK_MASK        GENMASK(31, 11)
>>   #define HISI_PTT_IS_8DW_PKT        GENMASK(31, 11)
>>   #define HISI_PTT_MAX_SPACE_LEN        10
>>   #define HISI_PTT_FIELD_LENGTH        4
>>   +/* Header DW0 fields for 4DW format */
>> +#define HISI_PTT_HEAD0_4DW_TIME        GENMASK_U32(10, 0)
>> +#define HISI_PTT_HEAD0_4DW_LEN        GENMASK_U32(20, 11)
>> +#define HISI_PTT_HEAD0_4DW_SO        BIT_U32(21)
>> +#define HISI_PTT_HEAD0_4DW_TH        BIT_U32(22)
>> +#define HISI_PTT_HEAD0_4DW_T8        BIT_U32(23)
>> +#define HISI_PTT_HEAD0_4DW_T9        BIT_U32(24)
>> +#define HISI_PTT_HEAD0_4DW_TYPE        GENMASK_U32(29, 25)
>> +#define HISI_PTT_HEAD0_4DW_FORMAT    GENMASK_U32(31, 30)
>> +
>>   enum hisi_ptt_pkt_type {
>>       HISI_PTT_4DW_PKT,
>>       HISI_PTT_8DW_PKT,
>
>