Re: [PATCH] perf arm-spe: Add support for SPE Data Source packet on HiSilicon HIP12

[Leo Yan <leo.yan@arm.com>]

Hi Yicong,

On Thu, Apr 24, 2025 at 07:57:39PM +0800, Yicong Yang wrote:

[...]

> >> If so, you might need to revise the cache levels for:
> >>
> >>   ARM_SPE_HISI_HIP_PEER_CPU
> >>   ARM_SPE_HISI_HIP_PEER_CPU_HITM
> >>
> >> IIUC, cluster CPUs share L3 cache, and every CPU in a cluster has
> >> L1/L2 cache, for PEER_CPU cases, the memory level should be L2.
> >>
> > 
> > confirmed with our hardware people, should be L2 for these two data sources.
> > I misunderstood here, thanks for pointing it out.
> 
> I recalled why the handling is like this. considering 2 threads have potential false
> sharing issues which are running on core0 thread0 and core 1 thread 0 in the same
> cluster, we'll have some ARM_SPE_HISI_HIP_PEER_CPU_HITM samples to indicate the
> cacheline contention. If the cache level is L2 then we cannot observe this by
> `perf c2c report -d tot`, since L2 is not counted for HITM.
> 
> does it make sense to have below change to account L2 hitm for lcl_hitm? just like
> we account L2 peer snoop for lcl_peer.

Yes.  The change below makes sense to me.

> diff --git a/tools/perf/util/mem-events.c b/tools/perf/util/mem-events.c
> index 884d9aebce91..a384a866a562 100644
> --- a/tools/perf/util/mem-events.c
> +++ b/tools/perf/util/mem-events.c
> @@ -680,7 +680,10 @@ do {                               \
>                         if (lvl & P(LVL, LFB)) stats->ld_fbhit++;
>                         if (lvl & P(LVL, L1 )) stats->ld_l1hit++;
>                         if (lvl & P(LVL, L2)) {
> -                               stats->ld_l2hit++;
> +                               if (snoop & P(SNOOP, HITM))
> +                                       HITM_INC(lcl_hitm);
> +                               else
> +                                       stats->ld_l2hit++;
> 
>                                 if (snoopx & P(SNOOPX, PEER))
>                                         PEER_INC(lcl_peer);