| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <7b6f3d35-02fc-46cc-9627-7ce748af5dac@linux.intel.com>
Date: Tue, 9 Jul 2024 08:56:49 -0400
From: "Liang, Kan" <kan.liang@...ux.intel.com>
To: Peter Zijlstra <peterz@...radead.org>
Cc: mingo@...nel.org, acme@...nel.org, namhyung@...nel.org,
irogers@...gle.com, adrian.hunter@...el.com,
alexander.shishkin@...ux.intel.com, linux-kernel@...r.kernel.org,
ak@...ux.intel.com, eranian@...gle.com, Ahmad Yasin <ahmad.yasin@...el.com>,
stable@...r.kernel.org
Subject: Re: [PATCH 2/3] perf/x86/intel: Add a distinct name for Granite
Rapids
On 2024-07-09 5:56 a.m., Peter Zijlstra wrote:
> On Mon, Jul 08, 2024 at 12:33:35PM -0700, kan.liang@...ux.intel.com wrote:
>> From: Kan Liang <kan.liang@...ux.intel.com>
>>
>> Currently, the Sapphire Rapids and Granite Rapids share the same PMU
>> name, sapphire_rapids. Because from the kernel’s perspective, GNR is
>> similar to SPR. The only key difference is that they support different
>> extra MSRs. The code path and the PMU name are shared.
>>
>> However, from end users' perspective, they are quite different. Besides
>> the extra MSRs, GNR has a newer PEBS format, supports Retire Latency,
>> supports new CPUID enumeration architecture, doesn't required the
>> load-latency AUX event, has additional TMA Level 1 Architectural Events,
>> etc. The differences can be enumerated by CPUID or the PERF_CAPABILITIES
>> MSR. They weren't reflected in the model-specific kernel setup.
>> But it is worth to have a distinct PMU name for GNR.
>>
>> Fixes: a6742cb90b56 ("perf/x86/intel: Fix the FRONTEND encoding on GNR and MTL")
>> Suggested-by: Ahmad Yasin <ahmad.yasin@...el.com>
>> Signed-off-by: Kan Liang <kan.liang@...ux.intel.com>
>> Cc: stable@...r.kernel.org
>> ---
>> arch/x86/events/intel/core.c | 9 ++++++---
>> 1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
>> index b61367991a16..7a9f931a1f48 100644
>> --- a/arch/x86/events/intel/core.c
>> +++ b/arch/x86/events/intel/core.c
>> @@ -6943,12 +6943,17 @@ __init int intel_pmu_init(void)
>> case INTEL_EMERALDRAPIDS_X:
>> x86_pmu.flags |= PMU_FL_MEM_LOADS_AUX;
>> x86_pmu.extra_regs = intel_glc_extra_regs;
>> + pr_cont("Sapphire Rapids events, ");
>> + name = "sapphire_rapids";
>> fallthrough;
>> case INTEL_GRANITERAPIDS_X:
>> case INTEL_GRANITERAPIDS_D:
>> intel_pmu_init_glc(NULL);
>> - if (!x86_pmu.extra_regs)
>> + if (!x86_pmu.extra_regs) {
>> x86_pmu.extra_regs = intel_rwc_extra_regs;
>> + pr_cont("Granite Rapids events, ");
>> + name = "granite_rapids";
>> + }
>> x86_pmu.pebs_ept = 1;
>> x86_pmu.hw_config = hsw_hw_config;
>> x86_pmu.get_event_constraints = glc_get_event_constraints;
>> @@ -6959,8 +6964,6 @@ __init int intel_pmu_init(void)
>> td_attr = glc_td_events_attrs;
>> tsx_attr = glc_tsx_events_attrs;
>> intel_pmu_pebs_data_source_skl(true);
>> - pr_cont("Sapphire Rapids events, ");
>> - name = "sapphire_rapids";
>> break;
>
> For some reason this didn't want to apply cleanly (something trivial),
> but since I had to edit it, my fingers slipped and I ended up with the
> below. That ok?
Yes, the patch looks good. Thanks!
Thanks,
Kan
>
> ---
> Subject: perf/x86/intel: Add a distinct name for Granite Rapids
> From: Kan Liang <kan.liang@...ux.intel.com>
> Date: Mon, 8 Jul 2024 12:33:35 -0700
>
> From: Kan Liang <kan.liang@...ux.intel.com>
>
> Currently, the Sapphire Rapids and Granite Rapids share the same PMU
> name, sapphire_rapids. Because from the kernel’s perspective, GNR is
> similar to SPR. The only key difference is that they support different
> extra MSRs. The code path and the PMU name are shared.
>
> However, from end users' perspective, they are quite different. Besides
> the extra MSRs, GNR has a newer PEBS format, supports Retire Latency,
> supports new CPUID enumeration architecture, doesn't required the
> load-latency AUX event, has additional TMA Level 1 Architectural Events,
> etc. The differences can be enumerated by CPUID or the PERF_CAPABILITIES
> MSR. They weren't reflected in the model-specific kernel setup.
> But it is worth to have a distinct PMU name for GNR.
>
> Fixes: a6742cb90b56 ("perf/x86/intel: Fix the FRONTEND encoding on GNR and MTL")
> Suggested-by: Ahmad Yasin <ahmad.yasin@...el.com>
> Signed-off-by: Kan Liang <kan.liang@...ux.intel.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
> Cc: stable@...r.kernel.org
> Link: https://lkml.kernel.org/r/20240708193336.1192217-3-kan.liang@linux.intel.com
> ---
> arch/x86/events/intel/core.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> --- a/arch/x86/events/intel/core.c
> +++ b/arch/x86/events/intel/core.c
> @@ -6788,12 +6788,18 @@ __init int intel_pmu_init(void)
> case INTEL_FAM6_EMERALDRAPIDS_X:
> x86_pmu.flags |= PMU_FL_MEM_LOADS_AUX;
> x86_pmu.extra_regs = intel_glc_extra_regs;
> - fallthrough;
> + pr_cont("Sapphire Rapids events, ");
> + name = "sapphire_rapids";
> + goto glc_common;
> +
> case INTEL_FAM6_GRANITERAPIDS_X:
> case INTEL_FAM6_GRANITERAPIDS_D:
> + x86_pmu.extra_regs = intel_rwc_extra_regs;
> + pr_cont("Granite Rapids events, ");
> + name = "granite_rapids";
> +
> + glc_common:
> intel_pmu_init_glc(NULL);
> - if (!x86_pmu.extra_regs)
> - x86_pmu.extra_regs = intel_rwc_extra_regs;
> x86_pmu.pebs_ept = 1;
> x86_pmu.hw_config = hsw_hw_config;
> x86_pmu.get_event_constraints = glc_get_event_constraints;
> @@ -6804,8 +6810,6 @@ __init int intel_pmu_init(void)
> td_attr = glc_td_events_attrs;
> tsx_attr = glc_tsx_events_attrs;
> intel_pmu_pebs_data_source_skl(true);
> - pr_cont("Sapphire Rapids events, ");
> - name = "sapphire_rapids";
> break;
>
> case INTEL_FAM6_ALDERLAKE:
>
Powered by blists - more mailing lists