[PATCH 0/1] perf cs-etm: Output 0 instead of 0xdeadbeef when exception packets are flushed

[PATCH 0/1] perf cs-etm: Output 0 instead of 0xdeadbeef when exception packets are flushed

[James Clark]

I noticed this when looking into Ganapat's fix to arm-cs-trace-disasm.py.
I assumed that seeing this in the output was a bug and went to
investigate why it happened in some cases and not others.

It turned out to not actually be a bug, but I think it doesn't look right.
In the end this change doesn't really accomplish anything and I'm not
sure if it's worth putting it in or not?

Maybe it will save someone doing the same thing as me, or maybe it will
actually break something if someones script is looking for 0xdeadbeef?

James Clark (1):
  perf cs-etm: Output 0 instead of 0xdeadbeef when exception packets are
    flushed

 tools/perf/util/cs-etm.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

-- 
2.34.1

[PATCH 1/1] perf cs-etm: Output 0 instead of 0xdeadbeef when exception packets are flushed

[James Clark]

Normally exception packets don't directly output a branch sample, but
if they're the last record in a buffer then they will. Because they
don't have addresses set we'll see the placeholder value
CS_ETM_INVAL_ADDR (0xdeadbeef) in the output.

Since commit 6035b6804bdf ("perf cs-etm: Support dummy address value for
CS_ETM_TRACE_ON packet") we've used 0 as an externally visible "not set"
address value. For consistency reasons and to not make exceptions look
like an error, change them to use 0 too.

This is particularly visible when doing userspace only tracing because
trace is disabled when jumping to the kernel, causing the flush and then
forcing the last exception packet to be emitted as a branch. With kernel
trace included, there is no flush so exception packets don't generate
samples until the next range packet and they'll pick up the correct
address.

Before:

  $ perf record -e cs_etm//u -- stress -i 1 -t 1
  $ perf script -F comm,ip,addr,flags

  stress   syscall                    ffffb7eedbc0 => deadbeefdeadbeef
  stress   syscall                    ffffb7f14a14 => deadbeefdeadbeef
  stress   syscall                    ffffb7eedbc0 => deadbeefdeadbeef

After:

  stress   syscall                    ffffb7eedbc0 =>                0
  stress   syscall                    ffffb7f14a14 =>                0
  stress   syscall                    ffffb7eedbc0 =>                0

Signed-off-by: James Clark <james.clark@linaro.org>
---
 tools/perf/util/cs-etm.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 5e9fbcfad7d4..d3e9c64d17d4 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -1267,8 +1267,12 @@ static inline int cs_etm__t32_instr_size(struct cs_etm_queue *etmq,
 
 static inline u64 cs_etm__first_executed_instr(struct cs_etm_packet *packet)
 {
-	/* Returns 0 for the CS_ETM_DISCONTINUITY packet */
-	if (packet->sample_type == CS_ETM_DISCONTINUITY)
+	/*
+	 * Return 0 for packets that have no addresses so that CS_ETM_INVAL_ADDR doesn't
+	 * appear in samples.
+	 */
+	if (packet->sample_type == CS_ETM_DISCONTINUITY ||
+	    packet->sample_type == CS_ETM_EXCEPTION)
 		return 0;
 
 	return packet->start_addr;
-- 
2.34.1

Re: [PATCH 1/1] perf cs-etm: Output 0 instead of 0xdeadbeef when exception packets are flushed

[Mike Leach]

On Mon, 22 Jul 2024 at 16:28, James Clark <james.clark@linaro.org> wrote:
>
> Normally exception packets don't directly output a branch sample, but
> if they're the last record in a buffer then they will. Because they
> don't have addresses set we'll see the placeholder value
> CS_ETM_INVAL_ADDR (0xdeadbeef) in the output.
>
> Since commit 6035b6804bdf ("perf cs-etm: Support dummy address value for
> CS_ETM_TRACE_ON packet") we've used 0 as an externally visible "not set"
> address value. For consistency reasons and to not make exceptions look
> like an error, change them to use 0 too.
>
> This is particularly visible when doing userspace only tracing because
> trace is disabled when jumping to the kernel, causing the flush and then
> forcing the last exception packet to be emitted as a branch. With kernel
> trace included, there is no flush so exception packets don't generate
> samples until the next range packet and they'll pick up the correct
> address.
>
> Before:
>
>   $ perf record -e cs_etm//u -- stress -i 1 -t 1
>   $ perf script -F comm,ip,addr,flags
>
>   stress   syscall                    ffffb7eedbc0 => deadbeefdeadbeef
>   stress   syscall                    ffffb7f14a14 => deadbeefdeadbeef
>   stress   syscall                    ffffb7eedbc0 => deadbeefdeadbeef
>
> After:
>
>   stress   syscall                    ffffb7eedbc0 =>                0
>   stress   syscall                    ffffb7f14a14 =>                0
>   stress   syscall                    ffffb7eedbc0 =>                0
>
> Signed-off-by: James Clark <james.clark@linaro.org>
> ---
>  tools/perf/util/cs-etm.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 5e9fbcfad7d4..d3e9c64d17d4 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -1267,8 +1267,12 @@ static inline int cs_etm__t32_instr_size(struct cs_etm_queue *etmq,
>
>  static inline u64 cs_etm__first_executed_instr(struct cs_etm_packet *packet)
>  {
> -       /* Returns 0 for the CS_ETM_DISCONTINUITY packet */
> -       if (packet->sample_type == CS_ETM_DISCONTINUITY)
> +       /*
> +        * Return 0 for packets that have no addresses so that CS_ETM_INVAL_ADDR doesn't
> +        * appear in samples.
> +        */
> +       if (packet->sample_type == CS_ETM_DISCONTINUITY ||
> +           packet->sample_type == CS_ETM_EXCEPTION)
>                 return 0;
>
>         return packet->start_addr;
> --
> 2.34.1
>

Reviewed-by: Mike Leach <mike.leach@linaro.org>

-- 
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK

Re: [PATCH 1/1] perf cs-etm: Output 0 instead of 0xdeadbeef when exception packets are flushed

[Arnaldo Carvalho de Melo]

On Tue, Jul 23, 2024 at 04:09:29PM +0100, Mike Leach wrote:
> On Mon, 22 Jul 2024 at 16:28, James Clark <james.clark@linaro.org> wrote:
> >
> > Normally exception packets don't directly output a branch sample, but
> > if they're the last record in a buffer then they will. Because they
> > don't have addresses set we'll see the placeholder value
> > CS_ETM_INVAL_ADDR (0xdeadbeef) in the output.
> >
> > Since commit 6035b6804bdf ("perf cs-etm: Support dummy address value for
> > CS_ETM_TRACE_ON packet") we've used 0 as an externally visible "not set"
> > address value. For consistency reasons and to not make exceptions look
> > like an error, change them to use 0 too.

<SNIP>
 
> Reviewed-by: Mike Leach <mike.leach@linaro.org>

Thanks, applied to tmp.perf-tools-next,

- Arnaldo