Re: [PATCH v9 09/12] perf/x86: Add INTEL_TD_METRIC_FIELD_{BITS,MASK} constants

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] perf/x86: conflation of UMASK shift and MSR metric field width
--

commit ca73afb21aaf968ca13fd3d0ba413055c3dc1861
Author: Zide Chen <zide.chen@intel.com>

perf/x86: Add INTEL_TD_METRIC_FIELD_{BITS,MASK} constants
    
Replace the hard coded field width (8) and the equivalent 0xff mask
used for PERF_METRICS MSR field access with named constants
INTEL_TD_METRIC_FIELD_{BITS,MASK}.

> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> index cc13164d948f2..cbc9a179f2e5c 100644
> --- a/arch/x86/events/intel/core.c
> +++ b/arch/x86/events/intel/core.c

[ ... ]

> @@ -4742,7 +4743,8 @@ static int core_pmu_hw_config(struct perf_event *event)
>  }
>  
>  #define INTEL_TD_METRIC_AVAILABLE_MAX	(INTEL_TD_METRIC_RETIRING + \
> -					 ((x86_pmu.num_topdown_events - 1) << 8))
> +					 ((x86_pmu.num_topdown_events - 1) << \
> +					  INTEL_TD_METRIC_FIELD_BITS))

[Severity: Low]
Is it safe to replace the UMASK shift here with the hardware MSR metric
field width?

The original << 8 was used to shift the metric ID into the UMASK field
(bits 8-15) of the software perf event configuration (attr.config).
INTEL_TD_METRIC_FIELD_BITS represents the hardware width of metrics within
the PERF_METRICS MSR.

When evaluated in is_available_metric_event() during perf_event_open(), this
currently functions because both values are 8. However, conflating a software
API layout offset with a hardware register field width could cause this macro
to calculate an invalid configuration mask if the MSR field width changes in
future hardware.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260918193937.569414-1-zide.chen@intel.com?part=9