Re: [PATCH v1 15/21] perf vendor events intel: Update novalake events from 1.00 to 1.04

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf: UMask omission in novalake events causes EventCode aliasing
--

commit 1470a977cdc98c6d093466522a50ce1b48974108
Author: Ian Rogers <irogers@google.com>

perf vendor events intel: Update novalake events from 1.00 to 1.04

This commit updates the novalake PMU events from version 1.00 to 1.04.
It synchronizes the event lists with the latest intel perfmon updates
and updates the format encoding for the off module response MSRs.

Link: https://github.com/intel/perfmon/commit/50afd00f84fb8c7bfa26a993a968982c1573e6d1
Link: https://github.com/intel/perfmon/commit/b576c542b68d377fa00ef4674715ea87a37c309c
Link: https://lore.kernel.org/linux-perf-users/20260923015517.2665398-1-dapeng1.mi@linux.intel.com/

> diff --git a/tools/perf/pmu-events/arch/x86/novalake/floating-point.json b/tools/perf/pmu-events/arch/x86/novalake/floating-point.json
> index 471a90f3670c4..2dda2225d0043 100644
> --- a/tools/perf/pmu-events/arch/x86/novalake/floating-point.json
> +++ b/tools/perf/pmu-events/arch/x86/novalake/floating-point.json

[ ... ]

> @@ -181,6 +181,22 @@
>          "UMask": "0x1",
>          "Unit": "cpu_core"
>      },
> +    {
> +        "BriefDescription": "FP_ARITH_INST_RETIRED.SCALAR_HALF",
> +        "Counter": "0,1,2,3,4,5,6,7",
> +        "EventCode": "0xc8",
> +        "EventName": "FP_ARITH_INST_RETIRED.SCALAR_HALF",
> +        "SampleAfterValue": "100003",
> +        "Unit": "cpu_core"
> +    },
> +    {
> +        "BriefDescription": "Number of all Scalar Half-Precision FP arithmetic instructions(1) retired - regular and complex.",
> +        "Counter": "0,1,2,3,4,5,6,7",
> +        "EventCode": "0xc8",
> +        "EventName": "FP_ARITH_INST_RETIRED.SCALAR_HALF_WCOMPLEX",
> +        "SampleAfterValue": "100003",
> +        "Unit": "cpu_core"
> +    },

[Severity: Medium]
Is the UMask intentionally omitted for FP_ARITH_INST_RETIRED.SCALAR_HALF_WCOMPLEX?

When the UMask is omitted, the perf tool JSON converter defaults to UMask 0x00.
Because FP_ARITH_INST_RETIRED.SCALAR_HALF also shares EventCode 0xc8 and
lacks a UMask, these become indistinguishable aliases for mutually exclusive
events.

Could this cause incorrect PMU programming and inaccurate measurements?

[ ... ]

> +    {
> +        "BriefDescription": "Counts the number of retired packed 128 bit or 256 bit double precision floating point arithmetic operations.",
> +        "Counter": "0,1,2,3,4,5,6,7",
> +        "EventCode": "0xc8",
> +        "EventName": "FP_ARITH_RETIRED.PACKED_DP",
> +        "SampleAfterValue": "1000003",
> +        "Unit": "cpu_atom"
> +    },

[Severity: Medium]
Similarly, FP_ARITH_RETIRED.PACKED_DP shares EventCode 0xc8 without a UMask.

This defaults to UMask 0x00, which will alias with PACKED_SP and other events
that share this EventCode.

Does this need a distinct UMask to prevent profiling collisions?

> diff --git a/tools/perf/pmu-events/arch/x86/novalake/pipeline.json b/tools/perf/pmu-events/arch/x86/novalake/pipeline.json
> index 998dd4cbc66a7..65a8a3058f81d 100644
> --- a/tools/perf/pmu-events/arch/x86/novalake/pipeline.json
> +++ b/tools/perf/pmu-events/arch/x86/novalake/pipeline.json

[ ... ]

> @@ -146,6 +196,15 @@
>          "UMask": "0x2",
>          "Unit": "cpu_core"
>      },
> +    {
> +        "BriefDescription": "Counts the number of far branch instructions retired, includes far jump, far call and return, and interrupt call and return.",
> +        "Counter": "0,1,2,3,4,5,6,7",
> +        "EventCode": "0xc4",
> +        "EventName": "BR_INST_RETIRED.FAR_BRANCH",
> +        "PublicDescription": "Counts the number of far branch instructions retired, includes far jump, far call and return, and interrupt call and return. Available PDIST counters: 0,1",
> +        "SampleAfterValue": "1000003",
> +        "Unit": "cpu_atom"
> +    },

[Severity: Medium]
Does BR_INST_RETIRED.FAR_BRANCH require a specific UMask?

It currently lacks one, defaulting to UMask 0x00 for cpu_atom. This will
collide with the preexisting ALL_BRANCHES event that also defaults to
UMask 0x00, leading to inaccurate performance measurements.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924053645.3555041-1-irogers@google.com?part=15