Re: [PATCH 1/7] perf/x86/intel: Support newly introduced 4 OMR MSRs for DMR & NVL

["Chen, Zide" <zide.chen@intel.com>]

On 11/19/2025 9:34 PM, Dapeng Mi wrote:
> Diamond Rapids and Nova Lake feature an expanded facility called
> the Off-Module Response (OMR) facility, which replaces the Off-Core
> Response (OCR) Performance Monitoring of previous processors.
> 
> Legacy microarchitectures used the OCR facility to evaluate off-core
> and multi-core off-module transactions. The properly renamed, OMR
> facility, improves the OCR capability for scalable coverage of new
> memory systems of multi-core module systems.
> 
> Similarly with OCR, 4 additional off-module configuration MSRs
> OFFMODULE_RSP_0 ~ OFFMODULE_RSP_3 are introduced to specify
> attributes of the off-module transaction.
> 
> For more details about OMR, please refer to section 16.1 "OFF-MODULE
>  RESPONSE (OMR) FACILITY" in ISE documentation.
> 
> This patch adds support for these 4 OMR events.
> 
> ISE link: https://www.intel.com/content/www/us/en/content-details/869288/intel-architecture-instruction-set-extensions-programming-reference.html
> 
> Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
> ---
>  arch/x86/events/intel/core.c     | 45 +++++++++++++++++++++++---------
>  arch/x86/events/perf_event.h     |  5 ++++
>  arch/x86/include/asm/msr-index.h |  5 ++++
>  3 files changed, 42 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> index aad89c9d9514..5970f7c20101 100644
> --- a/arch/x86/events/intel/core.c
> +++ b/arch/x86/events/intel/core.c
> @@ -3529,17 +3529,24 @@ static int intel_alt_er(struct cpu_hw_events *cpuc,
>  	struct extra_reg *extra_regs = hybrid(cpuc->pmu, extra_regs);
>  	int alt_idx = idx;
>  
> -	if (!(x86_pmu.flags & PMU_FL_HAS_RSP_1))
> -		return idx;
> -
> -	if (idx == EXTRA_REG_RSP_0)
> -		alt_idx = EXTRA_REG_RSP_1;
> -
> -	if (idx == EXTRA_REG_RSP_1)
> -		alt_idx = EXTRA_REG_RSP_0;
> +	if (idx == EXTRA_REG_RSP_0 || idx == EXTRA_REG_RSP_1) {
> +		if (!(x86_pmu.flags & PMU_FL_HAS_RSP_1))
> +			return idx;
> +		if (++alt_idx > EXTRA_REG_RSP_1)
> +			alt_idx = EXTRA_REG_RSP_0;
> +		if (config & ~extra_regs[alt_idx].valid_mask)
> +			return idx;
> +	}
>  
> -	if (config & ~extra_regs[alt_idx].valid_mask)
> -		return idx;
> +	if (idx >= EXTRA_REG_OMR_0 && idx <= EXTRA_REG_OMR_3) {
> +		if (!(x86_pmu.flags & PMU_FL_HAS_OMR))
> +			return idx;
> +		if (++alt_idx > EXTRA_REG_OMR_3)
> +			alt_idx = EXTRA_REG_OMR_0;
> +		if (config &
> +		    ~extra_regs[alt_idx - EXTRA_REG_OMR_0].valid_mask)

Why minus EXTRA_REG_OMR_0?

> +			return idx;
> +	}
>  
>  	return alt_idx;
>  }
> @@ -3547,16 +3554,28 @@ static int intel_alt_er(struct cpu_hw_events *cpuc,
>  static void intel_fixup_er(struct perf_event *event, int idx)
>  {
>  	struct extra_reg *extra_regs = hybrid(event->pmu, extra_regs);
> -	event->hw.extra_reg.idx = idx;
> +	int omr_idx;
>  
> -	if (idx == EXTRA_REG_RSP_0) {
> +	event->hw.extra_reg.idx = idx;
> +	switch (idx) {
> +	case EXTRA_REG_RSP_0:
>  		event->hw.config &= ~INTEL_ARCH_EVENT_MASK;
>  		event->hw.config |= extra_regs[EXTRA_REG_RSP_0].event;
>  		event->hw.extra_reg.reg = MSR_OFFCORE_RSP_0;
> -	} else if (idx == EXTRA_REG_RSP_1) {
> +		break;
> +	case EXTRA_REG_RSP_1:
>  		event->hw.config &= ~INTEL_ARCH_EVENT_MASK;
>  		event->hw.config |= extra_regs[EXTRA_REG_RSP_1].event;
>  		event->hw.extra_reg.reg = MSR_OFFCORE_RSP_1;

Keep same style?
case EXTRA_REG_RSP_0 ... EXTRA_REG_RSP_1:

> +		break;
> +	case EXTRA_REG_OMR_0 ... EXTRA_REG_OMR_3:
> +		omr_idx = idx - EXTRA_REG_OMR_0;
> +		event->hw.config &= ~ARCH_PERFMON_EVENTSEL_UMASK;
> +		event->hw.config |= 1ULL << (8 + omr_idx);
> +		event->hw.extra_reg.reg = MSR_OMR_0 + omr_idx;
> +		break;
> +	default:
> +		pr_warn("The extra reg idx %d is not supported.\n", idx);
>  	}
>  }
>