Linux Perf Users
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Sizhe Liu" <liusizhe5@huawei.com>
Cc: linux-perf-users@vger.kernel.org, linux-pci@vger.kernel.org
Subject: Re: [PATCH 02/10] perf hisi-ptt: Fix PTT trace TLP Header parsing
Date: Thu, 04 Jun 2026 07:59:58 +0000	[thread overview]
Message-ID: <20260604075958.A77611F00893@smtp.kernel.org> (raw)
In-Reply-To: <20260604075005.2219785-3-liusizhe5@huawei.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Relying on C bitfields without endianness macros breaks cross-platform decoding on big-endian hosts.
--

commit acedfe06d43ba61d4515f85b3f80767187e8bcb7
Author: Sizhe Liu <liusizhe5@huawei.com>

perf hisi-ptt: Fix PTT trace TLP Header parsing

This commit fixes the DW0 bit field layout of the hisi_ptt_4dw union to
match the actual bit ordering after little-endian memory extraction. It
also changes the hex value printout to big-endian for readability, aligning
with the format diagram.

> 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..67024f18ebbbc 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
> @@ -75,14 +76,14 @@ static const char * const hisi_ptt_4dw_pkt_field_name[] = {
>  
>  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 len : 10;
> +		uint32_t so : 1;
> +		uint32_t th : 1;
> +		uint32_t t8 : 1;
> +		uint32_t t9 : 1;
> +		uint32_t type : 5;
> +		uint32_t format : 2;
>  	};
>  	uint32_t value;
>  };

[Severity: Medium]
Does this bitfield layout break on big-endian hosts?

In C, bitfield memory layout within a struct is implementation-defined. GCC
packs bitfields from LSB to MSB on little-endian architectures, but from MSB
to LSB on big-endian architectures.

By unconditionally reordering the bitfields, the time: 11 field extracts the
lowest bits on little-endian hosts. However, on big-endian hosts, it will
incorrectly fetch bits 31-21 of the 32-bit native integer instead of bits 10-0,
resulting in corrupted trace decoding.

Should this struct use endianness macros (e.g., #if defined(__LITTLE_ENDIAN_BITFIELD)) 
or bitwise shifts instead of relying on C bitfields to ensure cross-platform 
decoding works correctly?

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

  reply	other threads:[~2026-06-04  7:59 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-04  7:49 [PATCH 00/10] perf hisi-ptt: Enhance TLP packet decoder with field-level parsing and versioning Sizhe Liu
2026-06-04  7:49 ` [PATCH 01/10] perf hisi-ptt: Fix spelling and abbreviation errors Sizhe Liu
2026-06-04  7:49 ` [PATCH 02/10] perf hisi-ptt: Fix PTT trace TLP Header parsing Sizhe Liu
2026-06-04  7:59   ` sashiko-bot [this message]
2026-06-04  7:49 ` [PATCH 03/10] perf hisi-ptt: Rename hisi_ptt_4dw union for reuse Sizhe Liu
2026-06-04  7:49 ` [PATCH 04/10] perf hisi-ptt: Abstract trace data buf and offset Sizhe Liu
2026-06-04  7:50 ` [PATCH 05/10] perf hisi-ptt: Complete the field names for 4DW and 8DW packets Sizhe Liu
2026-06-04  7:50 ` [PATCH 06/10] perf hisi-ptt: Extract the raw data printing part Sizhe Liu
2026-06-04  7:50 ` [PATCH 07/10] perf hisi-ptt: Merge 4DW and 8DW HEAD0 printing Sizhe Liu
2026-06-04  7:50 ` [PATCH 08/10] perf hisi-ptt: Add parsing of supported message types Sizhe Liu
2026-06-04  8:01   ` sashiko-bot
2026-06-04  7:50 ` [PATCH 09/10] perf hisi-ptt: Add field-level parsing for header DW2/DW3 Sizhe Liu
2026-06-04  7:58   ` sashiko-bot
2026-06-04  7:50 ` [PATCH 10/10] perf hisi-ptt: Add decoder version compatibility Sizhe Liu
2026-06-04  8:03   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260604075958.A77611F00893@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=liusizhe5@huawei.com \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox