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@xxxxxxxxxx> 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@xxxxxxxxxx>
> ---
>  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@xxxxxxxxxx>

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