Preparation for per-core energy counter support addition for AMD CPUs.
As there will always be just one rapl_model variable on a system, make it
global, to make it easier to access it from any function.
No functional change.
Signed-off-by: Dhananjay Ugwekar <Dhananjay.Ugwekar@amd.com>
---
arch/x86/events/rapl.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/arch/x86/events/rapl.c b/arch/x86/events/rapl.c
index a2f8c77fe629..46a6aeb61541 100644
--- a/arch/x86/events/rapl.c
+++ b/arch/x86/events/rapl.c
@@ -138,6 +138,7 @@ static struct rapl_pmus *rapl_pmus;
static unsigned int rapl_cntr_mask;
static u64 rapl_timer_ms;
static struct perf_msr *rapl_msrs;
+static struct rapl_model *rapl_model;
/*
* RAPL Package energy counter scope:
@@ -536,18 +537,18 @@ static struct perf_msr amd_rapl_msrs[] = {
[PERF_RAPL_PSYS] = { 0, &rapl_events_psys_group, NULL, false, 0 },
};
-static int rapl_check_hw_unit(struct rapl_model *rm)
+static int rapl_check_hw_unit(void)
{
u64 msr_rapl_power_unit_bits;
int i;
/* protect rdmsrl() to handle virtualization */
- if (rdmsrl_safe(rm->msr_power_unit, &msr_rapl_power_unit_bits))
+ if (rdmsrl_safe(rapl_model->msr_power_unit, &msr_rapl_power_unit_bits))
return -1;
for (i = 0; i < NR_RAPL_DOMAINS; i++)
rapl_hw_unit[i] = (msr_rapl_power_unit_bits >> 8) & 0x1FULL;
- switch (rm->unit_quirk) {
+ switch (rapl_model->unit_quirk) {
/*
* DRAM domain on HSW server and KNL has fixed energy unit which can be
* different than the unit from power unit MSR. See
@@ -798,21 +799,20 @@ MODULE_DEVICE_TABLE(x86cpu, rapl_model_match);
static int __init rapl_pmu_init(void)
{
const struct x86_cpu_id *id;
- struct rapl_model *rm;
int ret;
id = x86_match_cpu(rapl_model_match);
if (!id)
return -ENODEV;
- rm = (struct rapl_model *) id->driver_data;
+ rapl_model = (struct rapl_model *) id->driver_data;
- rapl_msrs = rm->rapl_msrs;
+ rapl_msrs = rapl_model->rapl_msrs;
rapl_cntr_mask = perf_msr_probe(rapl_msrs, PERF_RAPL_MAX,
- false, (void *) &rm->events);
+ false, (void *) &rapl_model->events);
- ret = rapl_check_hw_unit(rm);
+ ret = rapl_check_hw_unit();
if (ret)
return ret;
--
2.34.1Prep for per-core RAPL PMU addition.
No functional change.
Signed-off-by: Dhananjay Ugwekar <Dhananjay.Ugwekar@amd.com>
---
arch/x86/events/rapl.c | 32 +++++++++++++++++++-------------
1 file changed, 19 insertions(+), 13 deletions(-)
diff --git a/arch/x86/events/rapl.c b/arch/x86/events/rapl.c
index 46a6aeb61541..3bb1da8f96c3 100644
--- a/arch/x86/events/rapl.c
+++ b/arch/x86/events/rapl.c
@@ -597,7 +597,7 @@ static void __init rapl_advertise(void)
}
}
-static void cleanup_rapl_pmus(void)
+static void cleanup_rapl_pmus(struct rapl_pmus *rapl_pmus)
{
int i;
@@ -615,7 +615,7 @@ static const struct attribute_group *rapl_attr_update[] = {
NULL,
};
-static void __init init_rapl_pmu(void)
+static void __init init_rapl_pmu(struct rapl_pmus *rapl_pmus)
{
struct rapl_pmu *rapl_pmu;
int cpu, rapl_pmu_idx;
@@ -645,20 +645,22 @@ static void __init init_rapl_pmu(void)
cpus_read_unlock();
}
-static int __init init_rapl_pmus(void)
+static int __init init_rapl_pmus(struct rapl_pmus **rapl_pmus_ptr, int rapl_pmu_scope)
{
- int nr_rapl_pmu = topology_max_packages() * topology_max_dies_per_package();
- int rapl_pmu_scope = PERF_PMU_SCOPE_DIE;
+ int nr_rapl_pmu;
+ struct rapl_pmus *rapl_pmus;
- if (rapl_pmu_is_pkg_scope()) {
- nr_rapl_pmu = topology_max_packages();
- rapl_pmu_scope = PERF_PMU_SCOPE_PKG;
- }
+ if (rapl_pmu_scope == PERF_PMU_SCOPE_PKG)
+ nr_rapl_pmu = topology_max_packages();
+ else
+ nr_rapl_pmu = topology_max_packages() * topology_max_dies_per_package();
rapl_pmus = kzalloc(struct_size(rapl_pmus, rapl_pmu, nr_rapl_pmu), GFP_KERNEL);
if (!rapl_pmus)
return -ENOMEM;
+ *rapl_pmus_ptr = rapl_pmus;
+
rapl_pmus->nr_rapl_pmu = nr_rapl_pmu;
rapl_pmus->pmu.attr_groups = rapl_attr_groups;
rapl_pmus->pmu.attr_update = rapl_attr_update;
@@ -673,7 +675,7 @@ static int __init init_rapl_pmus(void)
rapl_pmus->pmu.module = THIS_MODULE;
rapl_pmus->pmu.capabilities = PERF_PMU_CAP_NO_EXCLUDE;
- init_rapl_pmu();
+ init_rapl_pmu(rapl_pmus);
return 0;
}
@@ -799,8 +801,12 @@ MODULE_DEVICE_TABLE(x86cpu, rapl_model_match);
static int __init rapl_pmu_init(void)
{
const struct x86_cpu_id *id;
+ int rapl_pmu_scope = PERF_PMU_SCOPE_DIE;
int ret;
+ if (rapl_pmu_is_pkg_scope())
+ rapl_pmu_scope = PERF_PMU_SCOPE_PKG;
+
id = x86_match_cpu(rapl_model_match);
if (!id)
return -ENODEV;
@@ -816,7 +822,7 @@ static int __init rapl_pmu_init(void)
if (ret)
return ret;
- ret = init_rapl_pmus();
+ ret = init_rapl_pmus(&rapl_pmus, rapl_pmu_scope);
if (ret)
return ret;
@@ -829,7 +835,7 @@ static int __init rapl_pmu_init(void)
out:
pr_warn("Initialization failed (%d), disabled\n", ret);
- cleanup_rapl_pmus();
+ cleanup_rapl_pmus(rapl_pmus);
return ret;
}
module_init(rapl_pmu_init);
@@ -837,6 +843,6 @@ module_init(rapl_pmu_init);
static void __exit intel_rapl_exit(void)
{
perf_pmu_unregister(&rapl_pmus->pmu);
- cleanup_rapl_pmus();
+ cleanup_rapl_pmus(rapl_pmus);
}
module_exit(intel_rapl_exit);
--
2.34.1Prep for addition of power_per_core PMU to handle core scope energy
consumption for AMD CPUs.
Replace the generic names with *_pkg*, to differentiate between the
scopes of the two different PMUs and their variables.
No functional change.
Signed-off-by: Dhananjay Ugwekar <Dhananjay.Ugwekar@amd.com>
---
arch/x86/events/rapl.c | 118 ++++++++++++++++++++---------------------
1 file changed, 59 insertions(+), 59 deletions(-)
diff --git a/arch/x86/events/rapl.c b/arch/x86/events/rapl.c
index 3bb1da8f96c3..023d208966bc 100644
--- a/arch/x86/events/rapl.c
+++ b/arch/x86/events/rapl.c
@@ -70,18 +70,18 @@ MODULE_LICENSE("GPL");
/*
* RAPL energy status counters
*/
-enum perf_rapl_events {
+enum perf_rapl_pkg_events {
PERF_RAPL_PP0 = 0, /* all cores */
PERF_RAPL_PKG, /* entire package */
PERF_RAPL_RAM, /* DRAM */
PERF_RAPL_PP1, /* gpu */
PERF_RAPL_PSYS, /* psys */
- PERF_RAPL_MAX,
- NR_RAPL_DOMAINS = PERF_RAPL_MAX,
+ PERF_RAPL_PKG_EVENTS_MAX,
+ NR_RAPL_PKG_DOMAINS = PERF_RAPL_PKG_EVENTS_MAX,
};
-static const char *const rapl_domain_names[NR_RAPL_DOMAINS] __initconst = {
+static const char *const rapl_pkg_domain_names[NR_RAPL_PKG_DOMAINS] __initconst = {
"pp0-core",
"package",
"dram",
@@ -126,16 +126,16 @@ enum rapl_unit_quirk {
};
struct rapl_model {
- struct perf_msr *rapl_msrs;
- unsigned long events;
+ struct perf_msr *rapl_pkg_msrs;
+ unsigned long pkg_events;
unsigned int msr_power_unit;
enum rapl_unit_quirk unit_quirk;
};
/* 1/2^hw_unit Joule */
-static int rapl_hw_unit[NR_RAPL_DOMAINS] __read_mostly;
-static struct rapl_pmus *rapl_pmus;
-static unsigned int rapl_cntr_mask;
+static int rapl_pkg_hw_unit[NR_RAPL_PKG_DOMAINS] __read_mostly;
+static struct rapl_pmus *rapl_pmus_pkg;
+static unsigned int rapl_pkg_cntr_mask;
static u64 rapl_timer_ms;
static struct perf_msr *rapl_msrs;
static struct rapl_model *rapl_model;
@@ -149,7 +149,7 @@ static struct rapl_model *rapl_model;
* considered as either pkg-scope or die-scope, and we are considering
* them as die-scope.
*/
-#define rapl_pmu_is_pkg_scope() \
+#define rapl_pkg_pmu_is_pkg_scope() \
(boot_cpu_data.x86_vendor == X86_VENDOR_AMD || \
boot_cpu_data.x86_vendor == X86_VENDOR_HYGON)
@@ -159,7 +159,7 @@ static struct rapl_model *rapl_model;
*/
static inline unsigned int get_rapl_pmu_idx(int cpu)
{
- return rapl_pmu_is_pkg_scope() ? topology_logical_package_id(cpu) :
+ return rapl_pkg_pmu_is_pkg_scope() ? topology_logical_package_id(cpu) :
topology_logical_die_id(cpu);
}
@@ -172,7 +172,7 @@ static inline u64 rapl_read_counter(struct perf_event *event)
static inline u64 rapl_scale(u64 v, int cfg)
{
- if (cfg > NR_RAPL_DOMAINS) {
+ if (cfg > NR_RAPL_PKG_DOMAINS) {
pr_warn("Invalid domain %d, failed to scale data\n", cfg);
return v;
}
@@ -182,7 +182,7 @@ static inline u64 rapl_scale(u64 v, int cfg)
* or use ldexp(count, -32).
* Watts = Joules/Time delta
*/
- return v << (32 - rapl_hw_unit[cfg - 1]);
+ return v << (32 - rapl_pkg_hw_unit[cfg - 1]);
}
static u64 rapl_event_update(struct perf_event *event)
@@ -342,7 +342,7 @@ static int rapl_pmu_event_init(struct perf_event *event)
struct rapl_pmu *rapl_pmu;
/* only look at RAPL events */
- if (event->attr.type != rapl_pmus->pmu.type)
+ if (event->attr.type != rapl_pmus_pkg->pmu.type)
return -ENOENT;
/* check only supported bits are set */
@@ -352,14 +352,14 @@ static int rapl_pmu_event_init(struct perf_event *event)
if (event->cpu < 0)
return -EINVAL;
- if (!cfg || cfg >= NR_RAPL_DOMAINS + 1)
+ if (!cfg || cfg >= NR_RAPL_PKG_DOMAINS + 1)
return -EINVAL;
- cfg = array_index_nospec((long)cfg, NR_RAPL_DOMAINS + 1);
+ cfg = array_index_nospec((long)cfg, NR_RAPL_PKG_DOMAINS + 1);
bit = cfg - 1;
/* check event supported */
- if (!(rapl_cntr_mask & (1 << bit)))
+ if (!(rapl_pkg_cntr_mask & (1 << bit)))
return -EINVAL;
/* unsupported modes and filters */
@@ -367,11 +367,11 @@ static int rapl_pmu_event_init(struct perf_event *event)
return -EINVAL;
rapl_pmu_idx = get_rapl_pmu_idx(event->cpu);
- if (rapl_pmu_idx >= rapl_pmus->nr_rapl_pmu)
+ if (rapl_pmu_idx >= rapl_pmus_pkg->nr_rapl_pmu)
return -EINVAL;
/* must be done before validate_group */
- rapl_pmu = rapl_pmus->rapl_pmu[rapl_pmu_idx];
+ rapl_pmu = rapl_pmus_pkg->rapl_pmu[rapl_pmu_idx];
if (!rapl_pmu)
return -EINVAL;
@@ -525,11 +525,11 @@ static struct perf_msr intel_rapl_spr_msrs[] = {
};
/*
- * Force to PERF_RAPL_MAX size due to:
- * - perf_msr_probe(PERF_RAPL_MAX)
+ * Force to PERF_RAPL_PKG_EVENTS_MAX size due to:
+ * - perf_msr_probe(PERF_RAPL_PKG_EVENTS_MAX)
* - want to use same event codes across both architectures
*/
-static struct perf_msr amd_rapl_msrs[] = {
+static struct perf_msr amd_rapl_pkg_msrs[] = {
[PERF_RAPL_PP0] = { 0, &rapl_events_cores_group, NULL, false, 0 },
[PERF_RAPL_PKG] = { MSR_AMD_PKG_ENERGY_STATUS, &rapl_events_pkg_group, test_msr, false, RAPL_MSR_MASK },
[PERF_RAPL_RAM] = { 0, &rapl_events_ram_group, NULL, false, 0 },
@@ -545,8 +545,8 @@ static int rapl_check_hw_unit(void)
/* protect rdmsrl() to handle virtualization */
if (rdmsrl_safe(rapl_model->msr_power_unit, &msr_rapl_power_unit_bits))
return -1;
- for (i = 0; i < NR_RAPL_DOMAINS; i++)
- rapl_hw_unit[i] = (msr_rapl_power_unit_bits >> 8) & 0x1FULL;
+ for (i = 0; i < NR_RAPL_PKG_DOMAINS; i++)
+ rapl_pkg_hw_unit[i] = (msr_rapl_power_unit_bits >> 8) & 0x1FULL;
switch (rapl_model->unit_quirk) {
/*
@@ -556,11 +556,11 @@ static int rapl_check_hw_unit(void)
* of 2. Datasheet, September 2014, Reference Number: 330784-001 "
*/
case RAPL_UNIT_QUIRK_INTEL_HSW:
- rapl_hw_unit[PERF_RAPL_RAM] = 16;
+ rapl_pkg_hw_unit[PERF_RAPL_RAM] = 16;
break;
/* SPR uses a fixed energy unit for Psys domain. */
case RAPL_UNIT_QUIRK_INTEL_SPR:
- rapl_hw_unit[PERF_RAPL_PSYS] = 0;
+ rapl_pkg_hw_unit[PERF_RAPL_PSYS] = 0;
break;
default:
break;
@@ -575,9 +575,9 @@ static int rapl_check_hw_unit(void)
* if hw unit is 32, then we use 2 ms 1/200/2
*/
rapl_timer_ms = 2;
- if (rapl_hw_unit[0] < 32) {
+ if (rapl_pkg_hw_unit[0] < 32) {
rapl_timer_ms = (1000 / (2 * 100));
- rapl_timer_ms *= (1ULL << (32 - rapl_hw_unit[0] - 1));
+ rapl_timer_ms *= (1ULL << (32 - rapl_pkg_hw_unit[0] - 1));
}
return 0;
}
@@ -587,12 +587,12 @@ static void __init rapl_advertise(void)
int i;
pr_info("API unit is 2^-32 Joules, %d fixed counters, %llu ms ovfl timer\n",
- hweight32(rapl_cntr_mask), rapl_timer_ms);
+ hweight32(rapl_pkg_cntr_mask), rapl_timer_ms);
- for (i = 0; i < NR_RAPL_DOMAINS; i++) {
- if (rapl_cntr_mask & (1 << i)) {
+ for (i = 0; i < NR_RAPL_PKG_DOMAINS; i++) {
+ if (rapl_pkg_cntr_mask & (1 << i)) {
pr_info("hw unit of domain %s 2^-%d Joules\n",
- rapl_domain_names[i], rapl_hw_unit[i]);
+ rapl_pkg_domain_names[i], rapl_pkg_hw_unit[i]);
}
}
}
@@ -681,71 +681,71 @@ static int __init init_rapl_pmus(struct rapl_pmus **rapl_pmus_ptr, int rapl_pmu_
}
static struct rapl_model model_snb = {
- .events = BIT(PERF_RAPL_PP0) |
+ .pkg_events = BIT(PERF_RAPL_PP0) |
BIT(PERF_RAPL_PKG) |
BIT(PERF_RAPL_PP1),
.msr_power_unit = MSR_RAPL_POWER_UNIT,
- .rapl_msrs = intel_rapl_msrs,
+ .rapl_pkg_msrs = intel_rapl_msrs,
};
static struct rapl_model model_snbep = {
- .events = BIT(PERF_RAPL_PP0) |
+ .pkg_events = BIT(PERF_RAPL_PP0) |
BIT(PERF_RAPL_PKG) |
BIT(PERF_RAPL_RAM),
.msr_power_unit = MSR_RAPL_POWER_UNIT,
- .rapl_msrs = intel_rapl_msrs,
+ .rapl_pkg_msrs = intel_rapl_msrs,
};
static struct rapl_model model_hsw = {
- .events = BIT(PERF_RAPL_PP0) |
+ .pkg_events = BIT(PERF_RAPL_PP0) |
BIT(PERF_RAPL_PKG) |
BIT(PERF_RAPL_RAM) |
BIT(PERF_RAPL_PP1),
.msr_power_unit = MSR_RAPL_POWER_UNIT,
- .rapl_msrs = intel_rapl_msrs,
+ .rapl_pkg_msrs = intel_rapl_msrs,
};
static struct rapl_model model_hsx = {
- .events = BIT(PERF_RAPL_PP0) |
+ .pkg_events = BIT(PERF_RAPL_PP0) |
BIT(PERF_RAPL_PKG) |
BIT(PERF_RAPL_RAM),
.unit_quirk = RAPL_UNIT_QUIRK_INTEL_HSW,
.msr_power_unit = MSR_RAPL_POWER_UNIT,
- .rapl_msrs = intel_rapl_msrs,
+ .rapl_pkg_msrs = intel_rapl_msrs,
};
static struct rapl_model model_knl = {
- .events = BIT(PERF_RAPL_PKG) |
+ .pkg_events = BIT(PERF_RAPL_PKG) |
BIT(PERF_RAPL_RAM),
.unit_quirk = RAPL_UNIT_QUIRK_INTEL_HSW,
.msr_power_unit = MSR_RAPL_POWER_UNIT,
- .rapl_msrs = intel_rapl_msrs,
+ .rapl_pkg_msrs = intel_rapl_msrs,
};
static struct rapl_model model_skl = {
- .events = BIT(PERF_RAPL_PP0) |
+ .pkg_events = BIT(PERF_RAPL_PP0) |
BIT(PERF_RAPL_PKG) |
BIT(PERF_RAPL_RAM) |
BIT(PERF_RAPL_PP1) |
BIT(PERF_RAPL_PSYS),
.msr_power_unit = MSR_RAPL_POWER_UNIT,
- .rapl_msrs = intel_rapl_msrs,
+ .rapl_pkg_msrs = intel_rapl_msrs,
};
static struct rapl_model model_spr = {
- .events = BIT(PERF_RAPL_PP0) |
+ .pkg_events = BIT(PERF_RAPL_PP0) |
BIT(PERF_RAPL_PKG) |
BIT(PERF_RAPL_RAM) |
BIT(PERF_RAPL_PSYS),
.unit_quirk = RAPL_UNIT_QUIRK_INTEL_SPR,
.msr_power_unit = MSR_RAPL_POWER_UNIT,
- .rapl_msrs = intel_rapl_spr_msrs,
+ .rapl_pkg_msrs = intel_rapl_spr_msrs,
};
static struct rapl_model model_amd_hygon = {
- .events = BIT(PERF_RAPL_PKG),
+ .pkg_events = BIT(PERF_RAPL_PKG),
.msr_power_unit = MSR_AMD_RAPL_POWER_UNIT,
- .rapl_msrs = amd_rapl_msrs,
+ .rapl_pkg_msrs = amd_rapl_pkg_msrs,
};
static const struct x86_cpu_id rapl_model_match[] __initconst = {
@@ -801,11 +801,11 @@ MODULE_DEVICE_TABLE(x86cpu, rapl_model_match);
static int __init rapl_pmu_init(void)
{
const struct x86_cpu_id *id;
- int rapl_pmu_scope = PERF_PMU_SCOPE_DIE;
+ int rapl_pkg_pmu_scope = PERF_PMU_SCOPE_DIE;
int ret;
- if (rapl_pmu_is_pkg_scope())
- rapl_pmu_scope = PERF_PMU_SCOPE_PKG;
+ if (rapl_pkg_pmu_is_pkg_scope())
+ rapl_pkg_pmu_scope = PERF_PMU_SCOPE_PKG;
id = x86_match_cpu(rapl_model_match);
if (!id)
@@ -813,20 +813,20 @@ static int __init rapl_pmu_init(void)
rapl_model = (struct rapl_model *) id->driver_data;
- rapl_msrs = rapl_model->rapl_msrs;
+ rapl_msrs = rapl_model->rapl_pkg_msrs;
- rapl_cntr_mask = perf_msr_probe(rapl_msrs, PERF_RAPL_MAX,
- false, (void *) &rapl_model->events);
+ rapl_pkg_cntr_mask = perf_msr_probe(rapl_msrs, PERF_RAPL_PKG_EVENTS_MAX,
+ false, (void *) &rapl_model->pkg_events);
ret = rapl_check_hw_unit();
if (ret)
return ret;
- ret = init_rapl_pmus(&rapl_pmus, rapl_pmu_scope);
+ ret = init_rapl_pmus(&rapl_pmus_pkg, rapl_pkg_pmu_scope);
if (ret)
return ret;
- ret = perf_pmu_register(&rapl_pmus->pmu, "power", -1);
+ ret = perf_pmu_register(&rapl_pmus_pkg->pmu, "power", -1);
if (ret)
goto out;
@@ -835,14 +835,14 @@ static int __init rapl_pmu_init(void)
out:
pr_warn("Initialization failed (%d), disabled\n", ret);
- cleanup_rapl_pmus(rapl_pmus);
+ cleanup_rapl_pmus(rapl_pmus_pkg);
return ret;
}
module_init(rapl_pmu_init);
static void __exit intel_rapl_exit(void)
{
- perf_pmu_unregister(&rapl_pmus->pmu);
- cleanup_rapl_pmus(rapl_pmus);
+ perf_pmu_unregister(&rapl_pmus_pkg->pmu);
+ cleanup_rapl_pmus(rapl_pmus_pkg);
}
module_exit(intel_rapl_exit);
--
2.34.1After making the rapl_model struct global, the rapl_msrs global
variable isn't needed, so remove it.
Also it will be cleaner when new per-core scope PMU is added. As we will
need to maintain two rapl_msrs array(one for per-core scope and one for
package scope PMU), inside the rapl_model struct.
Signed-off-by: Dhananjay Ugwekar <Dhananjay.Ugwekar@amd.com>
---
arch/x86/events/rapl.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/arch/x86/events/rapl.c b/arch/x86/events/rapl.c
index 023d208966bc..bfd8bbcc1421 100644
--- a/arch/x86/events/rapl.c
+++ b/arch/x86/events/rapl.c
@@ -137,7 +137,6 @@ static int rapl_pkg_hw_unit[NR_RAPL_PKG_DOMAINS] __read_mostly;
static struct rapl_pmus *rapl_pmus_pkg;
static unsigned int rapl_pkg_cntr_mask;
static u64 rapl_timer_ms;
-static struct perf_msr *rapl_msrs;
static struct rapl_model *rapl_model;
/*
@@ -376,7 +375,7 @@ static int rapl_pmu_event_init(struct perf_event *event)
return -EINVAL;
event->pmu_private = rapl_pmu;
- event->hw.event_base = rapl_msrs[bit].msr;
+ event->hw.event_base = rapl_model->rapl_pkg_msrs[bit].msr;
event->hw.config = cfg;
event->hw.idx = bit;
@@ -813,9 +812,7 @@ static int __init rapl_pmu_init(void)
rapl_model = (struct rapl_model *) id->driver_data;
- rapl_msrs = rapl_model->rapl_pkg_msrs;
-
- rapl_pkg_cntr_mask = perf_msr_probe(rapl_msrs, PERF_RAPL_PKG_EVENTS_MAX,
+ rapl_pkg_cntr_mask = perf_msr_probe(rapl_model->rapl_pkg_msrs, PERF_RAPL_PKG_EVENTS_MAX,
false, (void *) &rapl_model->pkg_events);
ret = rapl_check_hw_unit();
--
2.34.1Preparation for the addition of per-core RAPL energy counter for AMD
CPUs.
Moving cntr_mask to rapl_pmus struct instead of adding a new global
cntr_mask for the per-core RAPL energy counter, will ensure that the
"per_core_cntr_mask" is only created if needed (i.e. in case of AMD
CPUs).
Signed-off-by: Dhananjay Ugwekar <Dhananjay.Ugwekar@amd.com>
---
arch/x86/events/rapl.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/arch/x86/events/rapl.c b/arch/x86/events/rapl.c
index bfd8bbcc1421..7e9cb01f749a 100644
--- a/arch/x86/events/rapl.c
+++ b/arch/x86/events/rapl.c
@@ -116,6 +116,7 @@ struct rapl_pmu {
struct rapl_pmus {
struct pmu pmu;
unsigned int nr_rapl_pmu;
+ unsigned int cntr_mask;
struct rapl_pmu *rapl_pmu[] __counted_by(nr_rapl_pmu);
};
@@ -135,7 +136,6 @@ struct rapl_model {
/* 1/2^hw_unit Joule */
static int rapl_pkg_hw_unit[NR_RAPL_PKG_DOMAINS] __read_mostly;
static struct rapl_pmus *rapl_pmus_pkg;
-static unsigned int rapl_pkg_cntr_mask;
static u64 rapl_timer_ms;
static struct rapl_model *rapl_model;
@@ -358,7 +358,7 @@ static int rapl_pmu_event_init(struct perf_event *event)
bit = cfg - 1;
/* check event supported */
- if (!(rapl_pkg_cntr_mask & (1 << bit)))
+ if (!(rapl_pmus_pkg->cntr_mask & (1 << bit)))
return -EINVAL;
/* unsupported modes and filters */
@@ -586,10 +586,10 @@ static void __init rapl_advertise(void)
int i;
pr_info("API unit is 2^-32 Joules, %d fixed counters, %llu ms ovfl timer\n",
- hweight32(rapl_pkg_cntr_mask), rapl_timer_ms);
+ hweight32(rapl_pmus_pkg->cntr_mask), rapl_timer_ms);
for (i = 0; i < NR_RAPL_PKG_DOMAINS; i++) {
- if (rapl_pkg_cntr_mask & (1 << i)) {
+ if (rapl_pmus_pkg->cntr_mask & (1 << i)) {
pr_info("hw unit of domain %s 2^-%d Joules\n",
rapl_pkg_domain_names[i], rapl_pkg_hw_unit[i]);
}
@@ -812,9 +812,6 @@ static int __init rapl_pmu_init(void)
rapl_model = (struct rapl_model *) id->driver_data;
- rapl_pkg_cntr_mask = perf_msr_probe(rapl_model->rapl_pkg_msrs, PERF_RAPL_PKG_EVENTS_MAX,
- false, (void *) &rapl_model->pkg_events);
-
ret = rapl_check_hw_unit();
if (ret)
return ret;
@@ -823,6 +820,10 @@ static int __init rapl_pmu_init(void)
if (ret)
return ret;
+ rapl_pmus_pkg->cntr_mask = perf_msr_probe(rapl_model->rapl_pkg_msrs,
+ PERF_RAPL_PKG_EVENTS_MAX, false,
+ (void *) &rapl_model->pkg_events);
+
ret = perf_pmu_register(&rapl_pmus_pkg->pmu, "power", -1);
if (ret)
goto out;
--
2.34.1Add a new "power_per_core" PMU and "energy-per-core" event for
monitoring energy consumption by each core. The existing energy-cores
event aggregates the energy consumption at the package level.
This new event aligns with the AMD's per_core energy counters.
Tested the package level and core level PMU counters with workloads
pinned to different CPUs.
Results with workload pinned to CPU 1 in core 1 on a AMD Zen4 Genoa
machine:
$ perf stat -a --per-core -e power_per_core/energy-per-core/ sleep 1
Performance counter stats for 'system wide':
S0-D0-C0 1 0.02 Joules power_per_core/energy-per-core/
S0-D0-C1 1 5.72 Joules power_per_core/energy-per-core/
S0-D0-C2 1 0.02 Joules power_per_core/energy-per-core/
S0-D0-C3 1 0.02 Joules power_per_core/energy-per-core/
S0-D0-C4 1 0.02 Joules power_per_core/energy-per-core/
S0-D0-C5 1 0.02 Joules power_per_core/energy-per-core/
S0-D0-C6 1 0.02 Joules power_per_core/energy-per-core/
S0-D0-C7 1 0.02 Joules power_per_core/energy-per-core/
S0-D0-C8 1 0.02 Joules power_per_core/energy-per-core/
S0-D0-C9 1 0.02 Joules power_per_core/energy-per-core/
S0-D0-C10 1 0.02 Joules power_per_core/energy-per-core/
Signed-off-by: Dhananjay Ugwekar <Dhananjay.Ugwekar@amd.com>
---
v5 changes:
* Pass an extra "scope" argument to the get_rapl_pmu_idx() to
support the per-core PMU as well.
* Pass "perf_event" pointer argument to rapl_scale(), to use the
corresponding hw_unit value (pkg or core) to scale.
* Move around the code in rapl_pmu_event_init() to consolidate the PMU
specific code to single if block.
* Removed the sanity check in rapl_scale() as we have already
bound-checked "cfg" in rapl_pmu_event_init()
---
arch/x86/events/rapl.c | 178 +++++++++++++++++++++++++++++++++--------
1 file changed, 143 insertions(+), 35 deletions(-)
diff --git a/arch/x86/events/rapl.c b/arch/x86/events/rapl.c
index 7e9cb01f749a..6514ea67cf0c 100644
--- a/arch/x86/events/rapl.c
+++ b/arch/x86/events/rapl.c
@@ -39,6 +39,10 @@
* event: rapl_energy_psys
* perf code: 0x5
*
+ * per_core counter: consumption of a single physical core
+ * event: rapl_energy_per_core (power_per_core PMU)
+ * perf code: 0x1
+ *
* We manage those counters as free running (read-only). They may be
* use simultaneously by other tools, such as turbostat.
*
@@ -81,6 +85,10 @@ enum perf_rapl_pkg_events {
NR_RAPL_PKG_DOMAINS = PERF_RAPL_PKG_EVENTS_MAX,
};
+#define PERF_RAPL_PER_CORE 0 /* per-core */
+#define PERF_RAPL_CORE_EVENTS_MAX 1
+#define NR_RAPL_CORE_DOMAINS PERF_RAPL_CORE_EVENTS_MAX
+
static const char *const rapl_pkg_domain_names[NR_RAPL_PKG_DOMAINS] __initconst = {
"pp0-core",
"package",
@@ -89,6 +97,8 @@ static const char *const rapl_pkg_domain_names[NR_RAPL_PKG_DOMAINS] __initconst
"psys",
};
+static const char *const rapl_core_domain_name __initconst = "per-core";
+
/*
* event code: LSB 8 bits, passed in attr->config
* any other bit is reserved
@@ -128,14 +138,18 @@ enum rapl_unit_quirk {
struct rapl_model {
struct perf_msr *rapl_pkg_msrs;
+ struct perf_msr *rapl_core_msrs;
unsigned long pkg_events;
+ unsigned long core_events;
unsigned int msr_power_unit;
enum rapl_unit_quirk unit_quirk;
};
/* 1/2^hw_unit Joule */
static int rapl_pkg_hw_unit[NR_RAPL_PKG_DOMAINS] __read_mostly;
+static int rapl_core_hw_unit __read_mostly;
static struct rapl_pmus *rapl_pmus_pkg;
+static struct rapl_pmus *rapl_pmus_core;
static u64 rapl_timer_ms;
static struct rapl_model *rapl_model;
@@ -156,10 +170,14 @@ static struct rapl_model *rapl_model;
* Helper function to get the correct topology id according to the
* RAPL PMU scope.
*/
-static inline unsigned int get_rapl_pmu_idx(int cpu)
+static inline unsigned int get_rapl_pmu_idx(int cpu, int scope)
{
- return rapl_pkg_pmu_is_pkg_scope() ? topology_logical_package_id(cpu) :
- topology_logical_die_id(cpu);
+ if (scope == PERF_PMU_SCOPE_PKG)
+ return topology_logical_package_id(cpu);
+ else if (scope == PERF_PMU_SCOPE_DIE)
+ return topology_logical_die_id(cpu);
+ else
+ return topology_logical_core_id(cpu);
}
static inline u64 rapl_read_counter(struct perf_event *event)
@@ -169,19 +187,20 @@ static inline u64 rapl_read_counter(struct perf_event *event)
return raw;
}
-static inline u64 rapl_scale(u64 v, int cfg)
+static inline u64 rapl_scale(u64 v, struct perf_event *event)
{
- if (cfg > NR_RAPL_PKG_DOMAINS) {
- pr_warn("Invalid domain %d, failed to scale data\n", cfg);
- return v;
- }
+ int hw_unit = rapl_pkg_hw_unit[event->hw.config - 1];
+
+ if (event->pmu->scope == PERF_PMU_SCOPE_CORE)
+ hw_unit = rapl_core_hw_unit;
+
/*
* scale delta to smallest unit (1/2^32)
* users must then scale back: count * 1/(1e9*2^32) to get Joules
* or use ldexp(count, -32).
* Watts = Joules/Time delta
*/
- return v << (32 - rapl_pkg_hw_unit[cfg - 1]);
+ return v << (32 - hw_unit);
}
static u64 rapl_event_update(struct perf_event *event)
@@ -208,7 +227,7 @@ static u64 rapl_event_update(struct perf_event *event)
delta = (new_raw_count << shift) - (prev_raw_count << shift);
delta >>= shift;
- sdelta = rapl_scale(delta, event->hw.config);
+ sdelta = rapl_scale(delta, event);
local64_add(sdelta, &event->count);
@@ -337,12 +356,13 @@ static void rapl_pmu_event_del(struct perf_event *event, int flags)
static int rapl_pmu_event_init(struct perf_event *event)
{
u64 cfg = event->attr.config & RAPL_EVENT_MASK;
- int bit, rapl_pmu_idx, ret = 0;
+ int bit, rapl_pmus_scope, rapl_pmu_idx, ret = 0;
struct rapl_pmu *rapl_pmu;
+ struct rapl_pmus *rapl_pmus;
- /* only look at RAPL events */
- if (event->attr.type != rapl_pmus_pkg->pmu.type)
- return -ENOENT;
+ /* unsupported modes and filters */
+ if (event->attr.sample_period) /* no sampling */
+ return -EINVAL;
/* check only supported bits are set */
if (event->attr.config & ~RAPL_EVENT_MASK)
@@ -351,31 +371,49 @@ static int rapl_pmu_event_init(struct perf_event *event)
if (event->cpu < 0)
return -EINVAL;
- if (!cfg || cfg >= NR_RAPL_PKG_DOMAINS + 1)
+ rapl_pmus = container_of(event->pmu, struct rapl_pmus, pmu);
+ if (!rapl_pmus)
return -EINVAL;
-
- cfg = array_index_nospec((long)cfg, NR_RAPL_PKG_DOMAINS + 1);
- bit = cfg - 1;
-
- /* check event supported */
- if (!(rapl_pmus_pkg->cntr_mask & (1 << bit)))
+ rapl_pmus_scope = rapl_pmus->pmu.scope;
+
+ if (rapl_pmus_scope == PERF_PMU_SCOPE_PKG || rapl_pmus_scope == PERF_PMU_SCOPE_DIE) {
+ /* only look at RAPL package events */
+ if (event->attr.type != rapl_pmus_pkg->pmu.type)
+ return -ENOENT;
+
+ cfg = array_index_nospec((long)cfg, NR_RAPL_PKG_DOMAINS + 1);
+ if (!cfg || cfg >= NR_RAPL_PKG_DOMAINS + 1)
+ return -EINVAL;
+
+ bit = cfg - 1;
+ event->hw.event_base = rapl_model->rapl_pkg_msrs[bit].msr;
+ } else if (rapl_pmus_scope == PERF_PMU_SCOPE_CORE) {
+ /* only look at RAPL per-core events */
+ if (event->attr.type != rapl_pmus_core->pmu.type)
+ return -ENOENT;
+
+ cfg = array_index_nospec((long)cfg, NR_RAPL_CORE_DOMAINS + 1);
+ if (!cfg || cfg >= NR_RAPL_PKG_DOMAINS + 1)
+ return -EINVAL;
+
+ bit = cfg - 1;
+ event->hw.event_base = rapl_model->rapl_core_msrs[bit].msr;
+ } else
return -EINVAL;
- /* unsupported modes and filters */
- if (event->attr.sample_period) /* no sampling */
+ /* check event supported */
+ if (!(rapl_pmus->cntr_mask & (1 << bit)))
return -EINVAL;
- rapl_pmu_idx = get_rapl_pmu_idx(event->cpu);
- if (rapl_pmu_idx >= rapl_pmus_pkg->nr_rapl_pmu)
+ rapl_pmu_idx = get_rapl_pmu_idx(event->cpu, rapl_pmus_scope);
+ if (rapl_pmu_idx >= rapl_pmus->nr_rapl_pmu)
return -EINVAL;
-
/* must be done before validate_group */
- rapl_pmu = rapl_pmus_pkg->rapl_pmu[rapl_pmu_idx];
+ rapl_pmu = rapl_pmus->rapl_pmu[rapl_pmu_idx];
if (!rapl_pmu)
return -EINVAL;
event->pmu_private = rapl_pmu;
- event->hw.event_base = rapl_model->rapl_pkg_msrs[bit].msr;
event->hw.config = cfg;
event->hw.idx = bit;
@@ -392,12 +430,14 @@ RAPL_EVENT_ATTR_STR(energy-pkg , rapl_pkg, "event=0x02");
RAPL_EVENT_ATTR_STR(energy-ram , rapl_ram, "event=0x03");
RAPL_EVENT_ATTR_STR(energy-gpu , rapl_gpu, "event=0x04");
RAPL_EVENT_ATTR_STR(energy-psys, rapl_psys, "event=0x05");
+RAPL_EVENT_ATTR_STR(energy-per-core, rapl_per_core, "event=0x01");
RAPL_EVENT_ATTR_STR(energy-cores.unit, rapl_cores_unit, "Joules");
RAPL_EVENT_ATTR_STR(energy-pkg.unit , rapl_pkg_unit, "Joules");
RAPL_EVENT_ATTR_STR(energy-ram.unit , rapl_ram_unit, "Joules");
RAPL_EVENT_ATTR_STR(energy-gpu.unit , rapl_gpu_unit, "Joules");
RAPL_EVENT_ATTR_STR(energy-psys.unit, rapl_psys_unit, "Joules");
+RAPL_EVENT_ATTR_STR(energy-per-core.unit, rapl_per_core_unit, "Joules");
/*
* we compute in 0.23 nJ increments regardless of MSR
@@ -407,6 +447,7 @@ RAPL_EVENT_ATTR_STR(energy-pkg.scale, rapl_pkg_scale, "2.3283064365386962890
RAPL_EVENT_ATTR_STR(energy-ram.scale, rapl_ram_scale, "2.3283064365386962890625e-10");
RAPL_EVENT_ATTR_STR(energy-gpu.scale, rapl_gpu_scale, "2.3283064365386962890625e-10");
RAPL_EVENT_ATTR_STR(energy-psys.scale, rapl_psys_scale, "2.3283064365386962890625e-10");
+RAPL_EVENT_ATTR_STR(energy-per-core.scale, rapl_per_core_scale, "2.3283064365386962890625e-10");
/*
* There are no default events, but we need to create
@@ -439,6 +480,12 @@ static const struct attribute_group *rapl_attr_groups[] = {
NULL,
};
+static const struct attribute_group *rapl_per_core_attr_groups[] = {
+ &rapl_pmu_format_group,
+ &rapl_pmu_events_group,
+ NULL,
+};
+
static struct attribute *rapl_events_cores[] = {
EVENT_PTR(rapl_cores),
EVENT_PTR(rapl_cores_unit),
@@ -499,6 +546,18 @@ static struct attribute_group rapl_events_psys_group = {
.attrs = rapl_events_psys,
};
+static struct attribute *rapl_events_per_core[] = {
+ EVENT_PTR(rapl_per_core),
+ EVENT_PTR(rapl_per_core_unit),
+ EVENT_PTR(rapl_per_core_scale),
+ NULL,
+};
+
+static struct attribute_group rapl_events_per_core_group = {
+ .name = "events",
+ .attrs = rapl_events_per_core,
+};
+
static bool test_msr(int idx, void *data)
{
return test_bit(idx, (unsigned long *) data);
@@ -536,6 +595,11 @@ static struct perf_msr amd_rapl_pkg_msrs[] = {
[PERF_RAPL_PSYS] = { 0, &rapl_events_psys_group, NULL, false, 0 },
};
+static struct perf_msr amd_rapl_core_msrs[] = {
+ [PERF_RAPL_PER_CORE] = { MSR_AMD_CORE_ENERGY_STATUS, &rapl_events_per_core_group,
+ test_msr, false, RAPL_MSR_MASK },
+};
+
static int rapl_check_hw_unit(void)
{
u64 msr_rapl_power_unit_bits;
@@ -547,6 +611,8 @@ static int rapl_check_hw_unit(void)
for (i = 0; i < NR_RAPL_PKG_DOMAINS; i++)
rapl_pkg_hw_unit[i] = (msr_rapl_power_unit_bits >> 8) & 0x1FULL;
+ rapl_core_hw_unit = (msr_rapl_power_unit_bits >> 8) & 0x1FULL;
+
switch (rapl_model->unit_quirk) {
/*
* DRAM domain on HSW server and KNL has fixed energy unit which can be
@@ -565,7 +631,6 @@ static int rapl_check_hw_unit(void)
break;
}
-
/*
* Calculate the timer rate:
* Use reference of 200W for scaling the timeout to avoid counter
@@ -584,9 +649,13 @@ static int rapl_check_hw_unit(void)
static void __init rapl_advertise(void)
{
int i;
+ int num_counters = hweight32(rapl_pmus_pkg->cntr_mask);
+
+ if (rapl_pmus_core)
+ num_counters += hweight32(rapl_pmus_core->cntr_mask);
pr_info("API unit is 2^-32 Joules, %d fixed counters, %llu ms ovfl timer\n",
- hweight32(rapl_pmus_pkg->cntr_mask), rapl_timer_ms);
+ num_counters, rapl_timer_ms);
for (i = 0; i < NR_RAPL_PKG_DOMAINS; i++) {
if (rapl_pmus_pkg->cntr_mask & (1 << i)) {
@@ -594,6 +663,10 @@ static void __init rapl_advertise(void)
rapl_pkg_domain_names[i], rapl_pkg_hw_unit[i]);
}
}
+
+ if (rapl_pmus_core && (rapl_pmus_core->cntr_mask & (1 << PERF_RAPL_PER_CORE)))
+ pr_info("hw unit of domain %s 2^-%d Joules\n",
+ rapl_core_domain_name, rapl_core_hw_unit);
}
static void cleanup_rapl_pmus(struct rapl_pmus *rapl_pmus)
@@ -614,6 +687,10 @@ static const struct attribute_group *rapl_attr_update[] = {
NULL,
};
+static const struct attribute_group *rapl_per_core_attr_update[] = {
+ &rapl_events_per_core_group,
+};
+
static void __init init_rapl_pmu(struct rapl_pmus *rapl_pmus)
{
struct rapl_pmu *rapl_pmu;
@@ -622,10 +699,9 @@ static void __init init_rapl_pmu(struct rapl_pmus *rapl_pmus)
cpus_read_lock();
for_each_cpu(cpu, cpu_online_mask) {
- rapl_pmu_idx = get_rapl_pmu_idx(cpu);
+ rapl_pmu_idx = get_rapl_pmu_idx(cpu, rapl_pmus->pmu.scope);
if (rapl_pmu_idx >= rapl_pmus->nr_rapl_pmu)
continue;
-
rapl_pmu = rapl_pmus->rapl_pmu[rapl_pmu_idx];
if (rapl_pmu)
continue;
@@ -644,15 +720,19 @@ static void __init init_rapl_pmu(struct rapl_pmus *rapl_pmus)
cpus_read_unlock();
}
-static int __init init_rapl_pmus(struct rapl_pmus **rapl_pmus_ptr, int rapl_pmu_scope)
+static int __init init_rapl_pmus(struct rapl_pmus **rapl_pmus_ptr, int rapl_pmu_scope,
+ const struct attribute_group **rapl_attr_groups,
+ const struct attribute_group **rapl_attr_update)
{
int nr_rapl_pmu;
struct rapl_pmus *rapl_pmus;
if (rapl_pmu_scope == PERF_PMU_SCOPE_PKG)
nr_rapl_pmu = topology_max_packages();
- else
+ else if (rapl_pmu_scope == PERF_PMU_SCOPE_DIE)
nr_rapl_pmu = topology_max_packages() * topology_max_dies_per_package();
+ else
+ nr_rapl_pmu = topology_max_packages() * topology_num_cores_per_package();
rapl_pmus = kzalloc(struct_size(rapl_pmus, rapl_pmu, nr_rapl_pmu), GFP_KERNEL);
if (!rapl_pmus)
@@ -743,8 +823,10 @@ static struct rapl_model model_spr = {
static struct rapl_model model_amd_hygon = {
.pkg_events = BIT(PERF_RAPL_PKG),
+ .core_events = BIT(PERF_RAPL_PER_CORE),
.msr_power_unit = MSR_AMD_RAPL_POWER_UNIT,
.rapl_pkg_msrs = amd_rapl_pkg_msrs,
+ .rapl_core_msrs = amd_rapl_core_msrs,
};
static const struct x86_cpu_id rapl_model_match[] __initconst = {
@@ -816,7 +898,8 @@ static int __init rapl_pmu_init(void)
if (ret)
return ret;
- ret = init_rapl_pmus(&rapl_pmus_pkg, rapl_pkg_pmu_scope);
+ ret = init_rapl_pmus(&rapl_pmus_pkg, rapl_pkg_pmu_scope, rapl_attr_groups,
+ rapl_attr_update);
if (ret)
return ret;
@@ -828,6 +911,27 @@ static int __init rapl_pmu_init(void)
if (ret)
goto out;
+ if (rapl_model->core_events) {
+ ret = init_rapl_pmus(&rapl_pmus_core, PERF_PMU_SCOPE_CORE,
+ rapl_per_core_attr_groups,
+ rapl_per_core_attr_update);
+ if (ret) {
+ pr_warn("Per-core PMU initialization failed (%d)\n", ret);
+ goto per_core_init_failed;
+ }
+
+ rapl_pmus_core->cntr_mask = perf_msr_probe(rapl_model->rapl_core_msrs,
+ PERF_RAPL_CORE_EVENTS_MAX, false,
+ (void *) &rapl_model->core_events);
+
+ ret = perf_pmu_register(&rapl_pmus_core->pmu, "power_per_core", -1);
+ if (ret) {
+ pr_warn("Per-core PMU registration failed (%d)\n", ret);
+ cleanup_rapl_pmus(rapl_pmus_core);
+ }
+ }
+
+per_core_init_failed:
rapl_advertise();
return 0;
@@ -840,6 +944,10 @@ module_init(rapl_pmu_init);
static void __exit intel_rapl_exit(void)
{
+ if (rapl_pmus_core) {
+ perf_pmu_unregister(&rapl_pmus_core->pmu);
+ cleanup_rapl_pmus(rapl_pmus_core);
+ }
perf_pmu_unregister(&rapl_pmus_pkg->pmu);
cleanup_rapl_pmus(rapl_pmus_pkg);
}
--
2.34.1On Fri, Sep 13, 2024 at 03:48:01PM +0000, Dhananjay Ugwekar wrote:
> Add a new "power_per_core" PMU and "energy-per-core" event for
> monitoring energy consumption by each core. The existing energy-cores
> event aggregates the energy consumption at the package level.
> This new event aligns with the AMD's per_core energy counters.
>
> Tested the package level and core level PMU counters with workloads
> pinned to different CPUs.
>
> Results with workload pinned to CPU 1 in core 1 on a AMD Zen4 Genoa
> machine:
>
> $ perf stat -a --per-core -e power_per_core/energy-per-core/ sleep 1
>
> Performance counter stats for 'system wide':
>
> S0-D0-C0 1 0.02 Joules power_per_core/energy-per-core/
> S0-D0-C1 1 5.72 Joules power_per_core/energy-per-core/
> S0-D0-C2 1 0.02 Joules power_per_core/energy-per-core/
> S0-D0-C3 1 0.02 Joules power_per_core/energy-per-core/
> S0-D0-C4 1 0.02 Joules power_per_core/energy-per-core/
> S0-D0-C5 1 0.02 Joules power_per_core/energy-per-core/
> S0-D0-C6 1 0.02 Joules power_per_core/energy-per-core/
> S0-D0-C7 1 0.02 Joules power_per_core/energy-per-core/
> S0-D0-C8 1 0.02 Joules power_per_core/energy-per-core/
> S0-D0-C9 1 0.02 Joules power_per_core/energy-per-core/
> S0-D0-C10 1 0.02 Joules power_per_core/energy-per-core/
>
> Signed-off-by: Dhananjay Ugwekar <Dhananjay.Ugwekar@amd.com>
> ---
> v5 changes:
> * Pass an extra "scope" argument to the get_rapl_pmu_idx() to
> support the per-core PMU as well.
> * Pass "perf_event" pointer argument to rapl_scale(), to use the
> corresponding hw_unit value (pkg or core) to scale.
> * Move around the code in rapl_pmu_event_init() to consolidate the PMU
> specific code to single if block.
> * Removed the sanity check in rapl_scale() as we have already
> bound-checked "cfg" in rapl_pmu_event_init()
> ---
> arch/x86/events/rapl.c | 178 +++++++++++++++++++++++++++++++++--------
> 1 file changed, 143 insertions(+), 35 deletions(-)
>
> diff --git a/arch/x86/events/rapl.c b/arch/x86/events/rapl.c
> index 7e9cb01f749a..6514ea67cf0c 100644
> --- a/arch/x86/events/rapl.c
> +++ b/arch/x86/events/rapl.c
> @@ -39,6 +39,10 @@
> * event: rapl_energy_psys
> * perf code: 0x5
> *
> + * per_core counter: consumption of a single physical core
> + * event: rapl_energy_per_core (power_per_core PMU)
> + * perf code: 0x1
> + *
> * We manage those counters as free running (read-only). They may be
> * use simultaneously by other tools, such as turbostat.
> *
> @@ -81,6 +85,10 @@ enum perf_rapl_pkg_events {
> NR_RAPL_PKG_DOMAINS = PERF_RAPL_PKG_EVENTS_MAX,
> };
>
> +#define PERF_RAPL_PER_CORE 0 /* per-core */
> +#define PERF_RAPL_CORE_EVENTS_MAX 1
> +#define NR_RAPL_CORE_DOMAINS PERF_RAPL_CORE_EVENTS_MAX
> +
> static const char *const rapl_pkg_domain_names[NR_RAPL_PKG_DOMAINS] __initconst = {
> "pp0-core",
> "package",
> @@ -89,6 +97,8 @@ static const char *const rapl_pkg_domain_names[NR_RAPL_PKG_DOMAINS] __initconst
> "psys",
> };
>
> +static const char *const rapl_core_domain_name __initconst = "per-core";
> +
> /*
> * event code: LSB 8 bits, passed in attr->config
> * any other bit is reserved
> @@ -128,14 +138,18 @@ enum rapl_unit_quirk {
>
> struct rapl_model {
> struct perf_msr *rapl_pkg_msrs;
> + struct perf_msr *rapl_core_msrs;
> unsigned long pkg_events;
> + unsigned long core_events;
> unsigned int msr_power_unit;
> enum rapl_unit_quirk unit_quirk;
> };
>
> /* 1/2^hw_unit Joule */
> static int rapl_pkg_hw_unit[NR_RAPL_PKG_DOMAINS] __read_mostly;
> +static int rapl_core_hw_unit __read_mostly;
> static struct rapl_pmus *rapl_pmus_pkg;
> +static struct rapl_pmus *rapl_pmus_core;
> static u64 rapl_timer_ms;
> static struct rapl_model *rapl_model;
>
> @@ -156,10 +170,14 @@ static struct rapl_model *rapl_model;
> * Helper function to get the correct topology id according to the
> * RAPL PMU scope.
> */
> -static inline unsigned int get_rapl_pmu_idx(int cpu)
> +static inline unsigned int get_rapl_pmu_idx(int cpu, int scope)
> {
> - return rapl_pkg_pmu_is_pkg_scope() ? topology_logical_package_id(cpu) :
> - topology_logical_die_id(cpu);
> + if (scope == PERF_PMU_SCOPE_PKG)
> + return topology_logical_package_id(cpu);
> + else if (scope == PERF_PMU_SCOPE_DIE)
You don't need the "else if" since you are returning if there is a
match for the earlier if condition.
> + return topology_logical_die_id(cpu);
> + else
^^^^^
Please check if the scope is SCOPE_CORE here. Again, you don't need the
else condition.
> + return topology_logical_core_id(cpu);
> }
>
> static inline u64 rapl_read_counter(struct perf_event *event)
> @@ -169,19 +187,20 @@ static inline u64 rapl_read_counter(struct perf_event *event)
> return raw;
> }
>
> -static inline u64 rapl_scale(u64 v, int cfg)
> +static inline u64 rapl_scale(u64 v, struct perf_event *event)
> {
> - if (cfg > NR_RAPL_PKG_DOMAINS) {
> - pr_warn("Invalid domain %d, failed to scale data\n", cfg);
> - return v;
> - }
> + int hw_unit = rapl_pkg_hw_unit[event->hw.config - 1];
> +
> + if (event->pmu->scope == PERF_PMU_SCOPE_CORE)
> + hw_unit = rapl_core_hw_unit;
> +
> /*
> * scale delta to smallest unit (1/2^32)
> * users must then scale back: count * 1/(1e9*2^32) to get Joules
> * or use ldexp(count, -32).
> * Watts = Joules/Time delta
> */
> - return v << (32 - rapl_pkg_hw_unit[cfg - 1]);
> + return v << (32 - hw_unit);
> }
>
> static u64 rapl_event_update(struct perf_event *event)
> @@ -208,7 +227,7 @@ static u64 rapl_event_update(struct perf_event *event)
> delta = (new_raw_count << shift) - (prev_raw_count << shift);
> delta >>= shift;
>
> - sdelta = rapl_scale(delta, event->hw.config);
> + sdelta = rapl_scale(delta, event);
>
> local64_add(sdelta, &event->count);
>
> @@ -337,12 +356,13 @@ static void rapl_pmu_event_del(struct perf_event *event, int flags)
> static int rapl_pmu_event_init(struct perf_event *event)
> {
> u64 cfg = event->attr.config & RAPL_EVENT_MASK;
> - int bit, rapl_pmu_idx, ret = 0;
> + int bit, rapl_pmus_scope, rapl_pmu_idx, ret = 0;
> struct rapl_pmu *rapl_pmu;
> + struct rapl_pmus *rapl_pmus;
>
> - /* only look at RAPL events */
> - if (event->attr.type != rapl_pmus_pkg->pmu.type)
> - return -ENOENT;
Don't we need the check to only look at RAPL events of pkg or core ?
Or is that covered by a check below ?
> + /* unsupported modes and filters */
> + if (event->attr.sample_period) /* no sampling */
> + return -EINVAL;
>
> /* check only supported bits are set */
> if (event->attr.config & ~RAPL_EVENT_MASK)
> @@ -351,31 +371,49 @@ static int rapl_pmu_event_init(struct perf_event *event)
> if (event->cpu < 0)
> return -EINVAL;
>
> - if (!cfg || cfg >= NR_RAPL_PKG_DOMAINS + 1)
> + rapl_pmus = container_of(event->pmu, struct rapl_pmus, pmu);
> + if (!rapl_pmus)
> return -EINVAL;
> -
> - cfg = array_index_nospec((long)cfg, NR_RAPL_PKG_DOMAINS + 1);
> - bit = cfg - 1;
> -
> - /* check event supported */
> - if (!(rapl_pmus_pkg->cntr_mask & (1 << bit)))
> + rapl_pmus_scope = rapl_pmus->pmu.scope;
> +
> + if (rapl_pmus_scope == PERF_PMU_SCOPE_PKG || rapl_pmus_scope == PERF_PMU_SCOPE_DIE) {
> + /* only look at RAPL package events */
> + if (event->attr.type != rapl_pmus_pkg->pmu.type)
> + return -ENOENT;
> +
> + cfg = array_index_nospec((long)cfg, NR_RAPL_PKG_DOMAINS + 1);
> + if (!cfg || cfg >= NR_RAPL_PKG_DOMAINS + 1)
> + return -EINVAL;
> +
> + bit = cfg - 1;
> + event->hw.event_base = rapl_model->rapl_pkg_msrs[bit].msr;
> + } else if (rapl_pmus_scope == PERF_PMU_SCOPE_CORE) {
> + /* only look at RAPL per-core events */
> + if (event->attr.type != rapl_pmus_core->pmu.type)
> + return -ENOENT;
> +
> + cfg = array_index_nospec((long)cfg, NR_RAPL_CORE_DOMAINS + 1);
> + if (!cfg || cfg >= NR_RAPL_PKG_DOMAINS + 1)
> + return -EINVAL;
> +
> + bit = cfg - 1;
> + event->hw.event_base = rapl_model->rapl_core_msrs[bit].msr;
> + } else
> return -EINVAL;
>
> - /* unsupported modes and filters */
> - if (event->attr.sample_period) /* no sampling */
> + /* check event supported */
> + if (!(rapl_pmus->cntr_mask & (1 << bit)))
> return -EINVAL;
>
> - rapl_pmu_idx = get_rapl_pmu_idx(event->cpu);
> - if (rapl_pmu_idx >= rapl_pmus_pkg->nr_rapl_pmu)
> + rapl_pmu_idx = get_rapl_pmu_idx(event->cpu, rapl_pmus_scope);
> + if (rapl_pmu_idx >= rapl_pmus->nr_rapl_pmu)
> return -EINVAL;
> -
> /* must be done before validate_group */
> - rapl_pmu = rapl_pmus_pkg->rapl_pmu[rapl_pmu_idx];
> + rapl_pmu = rapl_pmus->rapl_pmu[rapl_pmu_idx];
> if (!rapl_pmu)
> return -EINVAL;
>
> event->pmu_private = rapl_pmu;
> - event->hw.event_base = rapl_model->rapl_pkg_msrs[bit].msr;
> event->hw.config = cfg;
> event->hw.idx = bit;
>
> @@ -392,12 +430,14 @@ RAPL_EVENT_ATTR_STR(energy-pkg , rapl_pkg, "event=0x02");
> RAPL_EVENT_ATTR_STR(energy-ram , rapl_ram, "event=0x03");
> RAPL_EVENT_ATTR_STR(energy-gpu , rapl_gpu, "event=0x04");
> RAPL_EVENT_ATTR_STR(energy-psys, rapl_psys, "event=0x05");
> +RAPL_EVENT_ATTR_STR(energy-per-core, rapl_per_core, "event=0x01");
>
> RAPL_EVENT_ATTR_STR(energy-cores.unit, rapl_cores_unit, "Joules");
> RAPL_EVENT_ATTR_STR(energy-pkg.unit , rapl_pkg_unit, "Joules");
> RAPL_EVENT_ATTR_STR(energy-ram.unit , rapl_ram_unit, "Joules");
> RAPL_EVENT_ATTR_STR(energy-gpu.unit , rapl_gpu_unit, "Joules");
> RAPL_EVENT_ATTR_STR(energy-psys.unit, rapl_psys_unit, "Joules");
> +RAPL_EVENT_ATTR_STR(energy-per-core.unit, rapl_per_core_unit, "Joules");
>
> /*
> * we compute in 0.23 nJ increments regardless of MSR
> @@ -407,6 +447,7 @@ RAPL_EVENT_ATTR_STR(energy-pkg.scale, rapl_pkg_scale, "2.3283064365386962890
> RAPL_EVENT_ATTR_STR(energy-ram.scale, rapl_ram_scale, "2.3283064365386962890625e-10");
> RAPL_EVENT_ATTR_STR(energy-gpu.scale, rapl_gpu_scale, "2.3283064365386962890625e-10");
> RAPL_EVENT_ATTR_STR(energy-psys.scale, rapl_psys_scale, "2.3283064365386962890625e-10");
> +RAPL_EVENT_ATTR_STR(energy-per-core.scale, rapl_per_core_scale, "2.3283064365386962890625e-10");
>
> /*
> * There are no default events, but we need to create
> @@ -439,6 +480,12 @@ static const struct attribute_group *rapl_attr_groups[] = {
> NULL,
> };
>
> +static const struct attribute_group *rapl_per_core_attr_groups[] = {
> + &rapl_pmu_format_group,
> + &rapl_pmu_events_group,
> + NULL,
> +};
> +
> static struct attribute *rapl_events_cores[] = {
> EVENT_PTR(rapl_cores),
> EVENT_PTR(rapl_cores_unit),
> @@ -499,6 +546,18 @@ static struct attribute_group rapl_events_psys_group = {
> .attrs = rapl_events_psys,
> };
>
> +static struct attribute *rapl_events_per_core[] = {
> + EVENT_PTR(rapl_per_core),
> + EVENT_PTR(rapl_per_core_unit),
> + EVENT_PTR(rapl_per_core_scale),
> + NULL,
> +};
> +
> +static struct attribute_group rapl_events_per_core_group = {
> + .name = "events",
> + .attrs = rapl_events_per_core,
> +};
> +
> static bool test_msr(int idx, void *data)
> {
> return test_bit(idx, (unsigned long *) data);
> @@ -536,6 +595,11 @@ static struct perf_msr amd_rapl_pkg_msrs[] = {
> [PERF_RAPL_PSYS] = { 0, &rapl_events_psys_group, NULL, false, 0 },
> };
>
> +static struct perf_msr amd_rapl_core_msrs[] = {
> + [PERF_RAPL_PER_CORE] = { MSR_AMD_CORE_ENERGY_STATUS, &rapl_events_per_core_group,
> + test_msr, false, RAPL_MSR_MASK },
> +};
> +
> static int rapl_check_hw_unit(void)
> {
> u64 msr_rapl_power_unit_bits;
> @@ -547,6 +611,8 @@ static int rapl_check_hw_unit(void)
> for (i = 0; i < NR_RAPL_PKG_DOMAINS; i++)
> rapl_pkg_hw_unit[i] = (msr_rapl_power_unit_bits >> 8) & 0x1FULL;
>
> + rapl_core_hw_unit = (msr_rapl_power_unit_bits >> 8) & 0x1FULL;
> +
> switch (rapl_model->unit_quirk) {
> /*
> * DRAM domain on HSW server and KNL has fixed energy unit which can be
> @@ -565,7 +631,6 @@ static int rapl_check_hw_unit(void)
> break;
> }
>
> -
> /*
> * Calculate the timer rate:
> * Use reference of 200W for scaling the timeout to avoid counter
> @@ -584,9 +649,13 @@ static int rapl_check_hw_unit(void)
> static void __init rapl_advertise(void)
> {
> int i;
> + int num_counters = hweight32(rapl_pmus_pkg->cntr_mask);
> +
> + if (rapl_pmus_core)
> + num_counters += hweight32(rapl_pmus_core->cntr_mask);
>
> pr_info("API unit is 2^-32 Joules, %d fixed counters, %llu ms ovfl timer\n",
> - hweight32(rapl_pmus_pkg->cntr_mask), rapl_timer_ms);
> + num_counters, rapl_timer_ms);
>
> for (i = 0; i < NR_RAPL_PKG_DOMAINS; i++) {
> if (rapl_pmus_pkg->cntr_mask & (1 << i)) {
> @@ -594,6 +663,10 @@ static void __init rapl_advertise(void)
> rapl_pkg_domain_names[i], rapl_pkg_hw_unit[i]);
> }
> }
> +
> + if (rapl_pmus_core && (rapl_pmus_core->cntr_mask & (1 << PERF_RAPL_PER_CORE)))
> + pr_info("hw unit of domain %s 2^-%d Joules\n",
> + rapl_core_domain_name, rapl_core_hw_unit);
> }
>
> static void cleanup_rapl_pmus(struct rapl_pmus *rapl_pmus)
> @@ -614,6 +687,10 @@ static const struct attribute_group *rapl_attr_update[] = {
> NULL,
> };
>
> +static const struct attribute_group *rapl_per_core_attr_update[] = {
> + &rapl_events_per_core_group,
> +};
> +
> static void __init init_rapl_pmu(struct rapl_pmus *rapl_pmus)
> {
> struct rapl_pmu *rapl_pmu;
> @@ -622,10 +699,9 @@ static void __init init_rapl_pmu(struct rapl_pmus *rapl_pmus)
> cpus_read_lock();
>
> for_each_cpu(cpu, cpu_online_mask) {
> - rapl_pmu_idx = get_rapl_pmu_idx(cpu);
> + rapl_pmu_idx = get_rapl_pmu_idx(cpu, rapl_pmus->pmu.scope);
> if (rapl_pmu_idx >= rapl_pmus->nr_rapl_pmu)
> continue;
> -
> rapl_pmu = rapl_pmus->rapl_pmu[rapl_pmu_idx];
> if (rapl_pmu)
> continue;
> @@ -644,15 +720,19 @@ static void __init init_rapl_pmu(struct rapl_pmus *rapl_pmus)
> cpus_read_unlock();
> }
>
> -static int __init init_rapl_pmus(struct rapl_pmus **rapl_pmus_ptr, int rapl_pmu_scope)
> +static int __init init_rapl_pmus(struct rapl_pmus **rapl_pmus_ptr, int rapl_pmu_scope,
> + const struct attribute_group **rapl_attr_groups,
> + const struct attribute_group **rapl_attr_update)
> {
> int nr_rapl_pmu;
> struct rapl_pmus *rapl_pmus;
>
> if (rapl_pmu_scope == PERF_PMU_SCOPE_PKG)
> nr_rapl_pmu = topology_max_packages();
> - else
> + else if (rapl_pmu_scope == PERF_PMU_SCOPE_DIE)
> nr_rapl_pmu = topology_max_packages() * topology_max_dies_per_package();
> + else
> + nr_rapl_pmu = topology_max_packages() * topology_num_cores_per_package();
Here please check if the rapl_pmu_scope is PERF_PMU_SCOPE_CORE instead
of assuming it to be the case if the scope is neither SCOPE_PKG nor
SCOPE_DIE. If it is neither of these three, then return an error.
--
Thanks and Regards
gautham.
>
> rapl_pmus = kzalloc(struct_size(rapl_pmus, rapl_pmu, nr_rapl_pmu), GFP_KERNEL);
> if (!rapl_pmus)
> @@ -743,8 +823,10 @@ static struct rapl_model model_spr = {
>
> static struct rapl_model model_amd_hygon = {
> .pkg_events = BIT(PERF_RAPL_PKG),
> + .core_events = BIT(PERF_RAPL_PER_CORE),
> .msr_power_unit = MSR_AMD_RAPL_POWER_UNIT,
> .rapl_pkg_msrs = amd_rapl_pkg_msrs,
> + .rapl_core_msrs = amd_rapl_core_msrs,
> };
>
> static const struct x86_cpu_id rapl_model_match[] __initconst = {
> @@ -816,7 +898,8 @@ static int __init rapl_pmu_init(void)
> if (ret)
> return ret;
>
> - ret = init_rapl_pmus(&rapl_pmus_pkg, rapl_pkg_pmu_scope);
> + ret = init_rapl_pmus(&rapl_pmus_pkg, rapl_pkg_pmu_scope, rapl_attr_groups,
> + rapl_attr_update);
> if (ret)
> return ret;
>
> @@ -828,6 +911,27 @@ static int __init rapl_pmu_init(void)
> if (ret)
> goto out;
>
> + if (rapl_model->core_events) {
> + ret = init_rapl_pmus(&rapl_pmus_core, PERF_PMU_SCOPE_CORE,
> + rapl_per_core_attr_groups,
> + rapl_per_core_attr_update);
> + if (ret) {
> + pr_warn("Per-core PMU initialization failed (%d)\n", ret);
> + goto per_core_init_failed;
> + }
> +
> + rapl_pmus_core->cntr_mask = perf_msr_probe(rapl_model->rapl_core_msrs,
> + PERF_RAPL_CORE_EVENTS_MAX, false,
> + (void *) &rapl_model->core_events);
> +
> + ret = perf_pmu_register(&rapl_pmus_core->pmu, "power_per_core", -1);
> + if (ret) {
> + pr_warn("Per-core PMU registration failed (%d)\n", ret);
> + cleanup_rapl_pmus(rapl_pmus_core);
> + }
> + }
> +
> +per_core_init_failed:
> rapl_advertise();
> return 0;
>
> @@ -840,6 +944,10 @@ module_init(rapl_pmu_init);
>
> static void __exit intel_rapl_exit(void)
> {
> + if (rapl_pmus_core) {
> + perf_pmu_unregister(&rapl_pmus_core->pmu);
> + cleanup_rapl_pmus(rapl_pmus_core);
> + }
> perf_pmu_unregister(&rapl_pmus_pkg->pmu);
> cleanup_rapl_pmus(rapl_pmus_pkg);
> }
> --
> 2.34.1
>
Hello Gautham,
On 10/7/2024 12:11 PM, Gautham R. Shenoy wrote:
> On Fri, Sep 13, 2024 at 03:48:01PM +0000, Dhananjay Ugwekar wrote:
>> Add a new "power_per_core" PMU and "energy-per-core" event for
>> monitoring energy consumption by each core. The existing energy-cores
>> event aggregates the energy consumption at the package level.
>> This new event aligns with the AMD's per_core energy counters.
[Snip]
>>
>> @@ -156,10 +170,14 @@ static struct rapl_model *rapl_model;
>> * Helper function to get the correct topology id according to the
>> * RAPL PMU scope.
>> */
>> -static inline unsigned int get_rapl_pmu_idx(int cpu)
>> +static inline unsigned int get_rapl_pmu_idx(int cpu, int scope)
>> {
>> - return rapl_pkg_pmu_is_pkg_scope() ? topology_logical_package_id(cpu) :
>> - topology_logical_die_id(cpu);
>> + if (scope == PERF_PMU_SCOPE_PKG)
>> + return topology_logical_package_id(cpu);
>> + else if (scope == PERF_PMU_SCOPE_DIE)
>
> You don't need the "else if" since you are returning if there is a
> match for the earlier if condition.
Right, will modify accordingly
>
>> + return topology_logical_die_id(cpu);
>> + else
> ^^^^^
> Please check if the scope is SCOPE_CORE here. Again, you don't need the
> else condition.
Yes, will add the if check and remove the else
>
>
>> + return topology_logical_core_id(cpu);
>
>
>
>> }
>>
>> static inline u64 rapl_read_counter(struct perf_event *event)
[Snip]
>> @@ -337,12 +356,13 @@ static void rapl_pmu_event_del(struct perf_event *event, int flags)
>> static int rapl_pmu_event_init(struct perf_event *event)
>> {
>> u64 cfg = event->attr.config & RAPL_EVENT_MASK;
>> - int bit, rapl_pmu_idx, ret = 0;
>> + int bit, rapl_pmus_scope, rapl_pmu_idx, ret = 0;
>> struct rapl_pmu *rapl_pmu;
>> + struct rapl_pmus *rapl_pmus;
>>
>> - /* only look at RAPL events */
>> - if (event->attr.type != rapl_pmus_pkg->pmu.type)
>> - return -ENOENT;
>
> Don't we need the check to only look at RAPL events of pkg or core ?
> Or is that covered by a check below ?
I moved this check to the PMU specific if blocks (highlighted below)
>
>
>
>> + /* unsupported modes and filters */
>> + if (event->attr.sample_period) /* no sampling */
>> + return -EINVAL;
>>
>> /* check only supported bits are set */
>> if (event->attr.config & ~RAPL_EVENT_MASK)
>> @@ -351,31 +371,49 @@ static int rapl_pmu_event_init(struct perf_event *event)
>> if (event->cpu < 0)
>> return -EINVAL;
>>
>> - if (!cfg || cfg >= NR_RAPL_PKG_DOMAINS + 1)
>> + rapl_pmus = container_of(event->pmu, struct rapl_pmus, pmu);
>> + if (!rapl_pmus)
>> return -EINVAL;
>> -
>> - cfg = array_index_nospec((long)cfg, NR_RAPL_PKG_DOMAINS + 1);
>> - bit = cfg - 1;
>> -
>> - /* check event supported */
>> - if (!(rapl_pmus_pkg->cntr_mask & (1 << bit)))
>> + rapl_pmus_scope = rapl_pmus->pmu.scope;
>> +
>> + if (rapl_pmus_scope == PERF_PMU_SCOPE_PKG || rapl_pmus_scope == PERF_PMU_SCOPE_DIE) {
>> + /* only look at RAPL package events */
>> + if (event->attr.type != rapl_pmus_pkg->pmu.type)
>> + return -ENOENT; ^^^^ here and
>> +
>> + cfg = array_index_nospec((long)cfg, NR_RAPL_PKG_DOMAINS + 1);
>> + if (!cfg || cfg >= NR_RAPL_PKG_DOMAINS + 1)
>> + return -EINVAL;
>> +
>> + bit = cfg - 1;
>> + event->hw.event_base = rapl_model->rapl_pkg_msrs[bit].msr;
>> + } else if (rapl_pmus_scope == PERF_PMU_SCOPE_CORE) {
>> + /* only look at RAPL per-core events */
>> + if (event->attr.type != rapl_pmus_core->pmu.type)
>> + return -ENOENT;
^^^^ here
>> +
>> + cfg = array_index_nospec((long)cfg, NR_RAPL_CORE_DOMAINS + 1);
>> + if (!cfg || cfg >= NR_RAPL_PKG_DOMAINS + 1)
>> + return -EINVAL;
>> +
>> + bit = cfg - 1;
>> + event->hw.event_base = rapl_model->rapl_core_msrs[bit].msr;
>> + } else
>> return -EINVAL;
>>
>> - /* unsupported modes and filters */
>> - if (event->attr.sample_period) /* no sampling */
>> + /* check event supported */
>> + if (!(rapl_pmus->cntr_mask & (1 << bit)))
>> return -EINVAL;
>>
>> - rapl_pmu_idx = get_rapl_pmu_idx(event->cpu);
>> - if (rapl_pmu_idx >= rapl_pmus_pkg->nr_rapl_pmu)
>> + rapl_pmu_idx = get_rapl_pmu_idx(event->cpu, rapl_pmus_scope);
>> + if (rapl_pmu_idx >= rapl_pmus->nr_rapl_pmu)
>> return -EINVAL;
>> -
>> /* must be done before validate_group */
>> - rapl_pmu = rapl_pmus_pkg->rapl_pmu[rapl_pmu_idx];
>> + rapl_pmu = rapl_pmus->rapl_pmu[rapl_pmu_idx];
>> if (!rapl_pmu)
>> return -EINVAL;
>>
>> event->pmu_private = rapl_pmu;
>> - event->hw.event_base = rapl_model->rapl_pkg_msrs[bit].msr;
>> event->hw.config = cfg;
>> event->hw.idx = bit;
>>
[Snip]
>> @@ -644,15 +720,19 @@ static void __init init_rapl_pmu(struct rapl_pmus *rapl_pmus)
>> cpus_read_unlock();
>> }
>>
>> -static int __init init_rapl_pmus(struct rapl_pmus **rapl_pmus_ptr, int rapl_pmu_scope)
>> +static int __init init_rapl_pmus(struct rapl_pmus **rapl_pmus_ptr, int rapl_pmu_scope,
>> + const struct attribute_group **rapl_attr_groups,
>> + const struct attribute_group **rapl_attr_update)
>> {
>> int nr_rapl_pmu;
>> struct rapl_pmus *rapl_pmus;
>>
>> if (rapl_pmu_scope == PERF_PMU_SCOPE_PKG)
>> nr_rapl_pmu = topology_max_packages();
>> - else
>> + else if (rapl_pmu_scope == PERF_PMU_SCOPE_DIE)
>> nr_rapl_pmu = topology_max_packages() * topology_max_dies_per_package();
>> + else
>> + nr_rapl_pmu = topology_max_packages() * topology_num_cores_per_package();
>
> Here please check if the rapl_pmu_scope is PERF_PMU_SCOPE_CORE instead
> of assuming it to be the case if the scope is neither SCOPE_PKG nor
> SCOPE_DIE. If it is neither of these three, then return an error.
Actually, I thought there is only one caller of this function (rapl_pmu_init),
which only passes one of these 3 values, so I thought maybe the error check isnt
needed. What do you think?
Thanks,
Dhananjay
>
>
> --
> Thanks and Regards
> gautham.
>
>>
>> rapl_pmus = kzalloc(struct_size(rapl_pmus, rapl_pmu, nr_rapl_pmu), GFP_KERNEL);
>> if (!rapl_pmus)
© 2016 - 2024 Red Hat, Inc.