| 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: <CAJZ5v0h4GAVCpUnbCMHMophsSZ522kkReAbUMdfFgRJfd0vHkw@mail.gmail.com>
Date: Fri, 2 Aug 2024 14:35:12 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Dhananjay Ugwekar <Dhananjay.Ugwekar@....com>
Cc: peterz@...radead.org, mingo@...hat.com, acme@...nel.org,
namhyung@...nel.org, mark.rutland@....com, alexander.shishkin@...ux.intel.com,
jolsa@...nel.org, irogers@...gle.com, adrian.hunter@...el.com,
kan.liang@...ux.intel.com, tglx@...utronix.de, bp@...en8.de,
dave.hansen@...ux.intel.com, x86@...nel.org, rui.zhang@...el.com,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-pm@...r.kernel.org, ananth.narayan@....com, gautham.shenoy@....com,
kprateek.nayak@....com, ravi.bangoria@....com,
Michael Larabel <michael@...haellarabel.com>
Subject: Re: [PATCH v2 2/2] powercap/intel_rapl: Fix the energy-pkg event for
AMD CPUs
On Tue, Jul 30, 2024 at 6:53 AM Dhananjay Ugwekar
<Dhananjay.Ugwekar@....com> wrote:
>
> After commit ("x86/cpu/topology: Add support for the AMD 0x80000026 leaf"),
> on AMD processors that support extended CPUID leaf 0x80000026, the
> topology_logical_die_id() macros, no longer returns package id, instead it
> returns the CCD (Core Complex Die) id. This leads to the energy-pkg
> event scope to be modified to CCD instead of package.
>
> For more historical context, please refer to commit 32fb480e0a2c
> ("powercap/intel_rapl: Support multi-die/package"), which initially changed
> the RAPL scope from package to die for all systems, as Intel systems
> with Die enumeration have RAPL scope as die, and those without die
> enumeration are not affected. So, all systems(Intel, AMD, Hygon), worked
> correctly with topology_logical_die_id() until recently, but this changed
> after the "0x80000026 leaf" commit mentioned above.
>
> Future multi-die Intel systems will have package scope RAPL counters,
> but they will be using TPMI RAPL interface, which is not affected by
> this change.
>
> Replacing topology_logical_die_id() with topology_physical_package_id()
> conditionally only for AMD and Hygon fixes the energy-pkg event.
>
> On an AMD 2 socket 8 CCD Zen4 server:
>
> Before:
>
> linux$ ls /sys/class/powercap/
> intel-rapl intel-rapl:4 intel-rapl:8:0 intel-rapl:d
> intel-rapl:0 intel-rapl:4:0 intel-rapl:9 intel-rapl:d:0
> intel-rapl:0:0 intel-rapl:5 intel-rapl:9:0 intel-rapl:e
> intel-rapl:1 intel-rapl:5:0 intel-rapl:a intel-rapl:e:0
> intel-rapl:1:0 intel-rapl:6 intel-rapl:a:0 intel-rapl:f
> intel-rapl:2 intel-rapl:6:0 intel-rapl:b intel-rapl:f:0
> intel-rapl:2:0 intel-rapl:7 intel-rapl:b:0
> intel-rapl:3 intel-rapl:7:0 intel-rapl:c
> intel-rapl:3:0 intel-rapl:8 intel-rapl:c:0
>
> After:
>
> linux$ ls /sys/class/powercap/
> intel-rapl intel-rapl:0 intel-rapl:0:0 intel-rapl:1 intel-rapl:1:0
>
> Only one sysfs entry per-event per-package is created after this change.
>
> Fixes: 63edbaa48a57 ("x86/cpu/topology: Add support for the AMD 0x80000026 leaf")
> Reported-by: Michael Larabel <michael@...haellarabel.com>
> Signed-off-by: Dhananjay Ugwekar <Dhananjay.Ugwekar@....com>
> Reviewed-by: Zhang Rui <rui.zhang@...el.com>
> ---
> Changes in v2:
> * Updated scope description comment, commit log
> * Rename rapl_pmu_is_pkg_scope() to rapl_msrs_are_pkg_scope()
> * Check topology_logical_(die/package)_id return value
This patch does not depend on the first one in the series if I'm not
mistaken, in which case I can pick it up separately if you want me to
do that, so please let me know.
Thanks!
> ---
> drivers/powercap/intel_rapl_common.c | 34 ++++++++++++++++++++++++----
> 1 file changed, 30 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/powercap/intel_rapl_common.c b/drivers/powercap/intel_rapl_common.c
> index 3cffa6c79538..4bc56acb99d6 100644
> --- a/drivers/powercap/intel_rapl_common.c
> +++ b/drivers/powercap/intel_rapl_common.c
> @@ -2128,6 +2128,21 @@ void rapl_remove_package(struct rapl_package *rp)
> }
> EXPORT_SYMBOL_GPL(rapl_remove_package);
>
> +/*
> + * RAPL Package energy counter scope:
> + * 1. AMD/HYGON platforms use per-PKG package energy counter
> + * 2. For Intel platforms
> + * 2.1 CLX-AP platform has per-DIE package energy counter
> + * 2.2 Other platforms that uses MSR RAPL are single die systems so the
> + * package energy counter can be considered as per-PKG/per-DIE,
> + * here it is considered as per-DIE.
> + * 2.3 New platforms that use TPMI RAPL doesn't care about the
> + * scope because they are not MSR/CPU based.
> + */
> +#define rapl_msrs_are_pkg_scope() \
> + (boot_cpu_data.x86_vendor == X86_VENDOR_AMD || \
> + boot_cpu_data.x86_vendor == X86_VENDOR_HYGON)
> +
> /* caller to ensure CPU hotplug lock is held */
> struct rapl_package *rapl_find_package_domain_cpuslocked(int id, struct rapl_if_priv *priv,
> bool id_is_cpu)
> @@ -2135,8 +2150,14 @@ struct rapl_package *rapl_find_package_domain_cpuslocked(int id, struct rapl_if_
> struct rapl_package *rp;
> int uid;
>
> - if (id_is_cpu)
> - uid = topology_logical_die_id(id);
> + if (id_is_cpu) {
> + uid = rapl_msrs_are_pkg_scope() ?
> + topology_physical_package_id(id) : topology_logical_die_id(id);
> + if (uid < 0) {
> + pr_err("topology_logical_(package/die)_id() returned a negative value");
> + return ERR_PTR(-EINVAL);
> + }
> + }
> else
> uid = id;
>
> @@ -2168,9 +2189,14 @@ struct rapl_package *rapl_add_package_cpuslocked(int id, struct rapl_if_priv *pr
> return ERR_PTR(-ENOMEM);
>
> if (id_is_cpu) {
> - rp->id = topology_logical_die_id(id);
> + rp->id = rapl_msrs_are_pkg_scope() ?
> + topology_physical_package_id(id) : topology_logical_die_id(id);
> + if ((int)(rp->id) < 0) {
> + pr_err("topology_logical_(package/die)_id() returned a negative value");
> + return ERR_PTR(-EINVAL);
> + }
> rp->lead_cpu = id;
> - if (topology_max_dies_per_package() > 1)
> + if (!rapl_msrs_are_pkg_scope() && topology_max_dies_per_package() > 1)
> snprintf(rp->name, PACKAGE_DOMAIN_NAME_LENGTH, "package-%d-die-%d",
> topology_physical_package_id(id), topology_die_id(id));
> else
> --
> 2.43.0
>
>
Powered by blists - more mailing lists