* [PATCH 1/3] perf/x86/rapl: move RAPL support to common x86 code
2020-05-15 21:57 [PATCH 0/3] perf/x86/rapl: Enable RAPL for AMD Fam17h Stephane Eranian
@ 2020-05-15 21:57 ` Stephane Eranian
2020-05-18 9:08 ` Peter Zijlstra
2020-05-15 21:57 ` [PATCH 2/3] perf/x86/rapl: refactor code for Intel/AMD sharing Stephane Eranian
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Stephane Eranian @ 2020-05-15 21:57 UTC (permalink / raw)
To: linux-kernel; +Cc: peterz, mingo, irogers, kim.phillips, jolsa
To prepare for support of both Intel and AMD RAPL.
Move rapl.c to arch/x86/events. Rename config option.
Fixup header paths.
Signed-off-by: Stephane Eranian <eranian@google.com>
---
arch/x86/events/Kconfig | 8 ++++----
arch/x86/events/Makefile | 1 +
arch/x86/events/intel/Makefile | 2 --
arch/x86/events/{intel => }/rapl.c | 9 ++++++---
4 files changed, 11 insertions(+), 9 deletions(-)
rename arch/x86/events/{intel => }/rapl.c (98%)
diff --git a/arch/x86/events/Kconfig b/arch/x86/events/Kconfig
index 9a7a1446cb3a0..e542c32b0a55f 100644
--- a/arch/x86/events/Kconfig
+++ b/arch/x86/events/Kconfig
@@ -9,12 +9,12 @@ config PERF_EVENTS_INTEL_UNCORE
Include support for Intel uncore performance events. These are
available on NehalemEX and more modern processors.
-config PERF_EVENTS_INTEL_RAPL
- tristate "Intel rapl performance events"
- depends on PERF_EVENTS && CPU_SUP_INTEL && PCI
+config PERF_EVENTS_X86_RAPL
+ tristate "RAPL performance events"
+ depends on PERF_EVENTS && (CPU_SUP_INTEL || CPU_SUP_AMD) && PCI
default y
---help---
- Include support for Intel rapl performance events for power
+ Include support for Intel and AMD rapl performance events for power
monitoring on modern processors.
config PERF_EVENTS_INTEL_CSTATE
diff --git a/arch/x86/events/Makefile b/arch/x86/events/Makefile
index 6f1d1fde8b2de..d5087a5745108 100644
--- a/arch/x86/events/Makefile
+++ b/arch/x86/events/Makefile
@@ -1,5 +1,6 @@
# SPDX-License-Identifier: GPL-2.0-only
obj-y += core.o probe.o
+obj-$(CONFIG_PERF_EVENTS_X86_RAPL) += rapl.o
obj-y += amd/
obj-$(CONFIG_X86_LOCAL_APIC) += msr.o
obj-$(CONFIG_CPU_SUP_INTEL) += intel/
diff --git a/arch/x86/events/intel/Makefile b/arch/x86/events/intel/Makefile
index 3468b0c1dc7c9..e67a5886336c1 100644
--- a/arch/x86/events/intel/Makefile
+++ b/arch/x86/events/intel/Makefile
@@ -2,8 +2,6 @@
obj-$(CONFIG_CPU_SUP_INTEL) += core.o bts.o
obj-$(CONFIG_CPU_SUP_INTEL) += ds.o knc.o
obj-$(CONFIG_CPU_SUP_INTEL) += lbr.o p4.o p6.o pt.o
-obj-$(CONFIG_PERF_EVENTS_INTEL_RAPL) += intel-rapl-perf.o
-intel-rapl-perf-objs := rapl.o
obj-$(CONFIG_PERF_EVENTS_INTEL_UNCORE) += intel-uncore.o
intel-uncore-objs := uncore.o uncore_nhmex.o uncore_snb.o uncore_snbep.o
obj-$(CONFIG_PERF_EVENTS_INTEL_CSTATE) += intel-cstate.o
diff --git a/arch/x86/events/intel/rapl.c b/arch/x86/events/rapl.c
similarity index 98%
rename from arch/x86/events/intel/rapl.c
rename to arch/x86/events/rapl.c
index a5dbd25852cb7..ece043fb7b494 100644
--- a/arch/x86/events/intel/rapl.c
+++ b/arch/x86/events/rapl.c
@@ -1,11 +1,14 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
- * Support Intel RAPL energy consumption counters
+ * Support Intel/AMD RAPL energy consumption counters
* Copyright (C) 2013 Google, Inc., Stephane Eranian
*
* Intel RAPL interface is specified in the IA-32 Manual Vol3b
* section 14.7.1 (September 2013)
*
+ * AMD RAPL interface for Fam17h is described in the public PPR:
+ * https://bugzilla.kernel.org/show_bug.cgi?id=206537
+ *
* RAPL provides more controls than just reporting energy consumption
* however here we only expose the 3 energy consumption free running
* counters (pp0, pkg, dram).
@@ -58,8 +61,8 @@
#include <linux/nospec.h>
#include <asm/cpu_device_id.h>
#include <asm/intel-family.h>
-#include "../perf_event.h"
-#include "../probe.h"
+#include "perf_event.h"
+#include "probe.h"
MODULE_LICENSE("GPL");
--
2.26.2.761.g0e0b3e54be-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 1/3] perf/x86/rapl: move RAPL support to common x86 code
2020-05-15 21:57 ` [PATCH 1/3] perf/x86/rapl: move RAPL support to common x86 code Stephane Eranian
@ 2020-05-18 9:08 ` Peter Zijlstra
0 siblings, 0 replies; 11+ messages in thread
From: Peter Zijlstra @ 2020-05-18 9:08 UTC (permalink / raw)
To: Stephane Eranian; +Cc: linux-kernel, mingo, irogers, kim.phillips, jolsa
On Fri, May 15, 2020 at 02:57:31PM -0700, Stephane Eranian wrote:
> To prepare for support of both Intel and AMD RAPL.
> Move rapl.c to arch/x86/events. Rename config option.
> Fixup header paths.
>
> Signed-off-by: Stephane Eranian <eranian@google.com>
> ---
> arch/x86/events/Kconfig | 8 ++++----
> arch/x86/events/Makefile | 1 +
> arch/x86/events/intel/Makefile | 2 --
> arch/x86/events/{intel => }/rapl.c | 9 ++++++---
> 4 files changed, 11 insertions(+), 9 deletions(-)
> rename arch/x86/events/{intel => }/rapl.c (98%)
>
> diff --git a/arch/x86/events/Kconfig b/arch/x86/events/Kconfig
> index 9a7a1446cb3a0..e542c32b0a55f 100644
> --- a/arch/x86/events/Kconfig
> +++ b/arch/x86/events/Kconfig
> @@ -9,12 +9,12 @@ config PERF_EVENTS_INTEL_UNCORE
> Include support for Intel uncore performance events. These are
> available on NehalemEX and more modern processors.
>
> -config PERF_EVENTS_INTEL_RAPL
> - tristate "Intel rapl performance events"
> - depends on PERF_EVENTS && CPU_SUP_INTEL && PCI
> +config PERF_EVENTS_X86_RAPL
This is going to make everybody's kconfig prompt for this again. Best to
to a backwards compat thing or just leave it have Intel in the name.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/3] perf/x86/rapl: refactor code for Intel/AMD sharing
2020-05-15 21:57 [PATCH 0/3] perf/x86/rapl: Enable RAPL for AMD Fam17h Stephane Eranian
2020-05-15 21:57 ` [PATCH 1/3] perf/x86/rapl: move RAPL support to common x86 code Stephane Eranian
@ 2020-05-15 21:57 ` Stephane Eranian
2020-05-18 9:23 ` Peter Zijlstra
2020-05-15 21:57 ` [PATCH 3/3] perf/x86/rapl: add AMD Fam17h RAPL support Stephane Eranian
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Stephane Eranian @ 2020-05-15 21:57 UTC (permalink / raw)
To: linux-kernel; +Cc: peterz, mingo, irogers, kim.phillips, jolsa
This patch modifies the rapl_model struct to include architecture specific
knowledge to Intel specific structure, and in particular the MSR for
POWER_UNIT and the rapl_msrs array.
No functional changes.
Signed-off-by: Stephane Eranian <eranian@google.com>
---
arch/x86/events/rapl.c | 29 +++++++++++++++++++++++------
1 file changed, 23 insertions(+), 6 deletions(-)
diff --git a/arch/x86/events/rapl.c b/arch/x86/events/rapl.c
index ece043fb7b494..e98f627a13fa8 100644
--- a/arch/x86/events/rapl.c
+++ b/arch/x86/events/rapl.c
@@ -131,7 +131,9 @@ struct rapl_pmus {
};
struct rapl_model {
+ struct perf_msr *rapl_msrs;
unsigned long events;
+ int msr_power_unit;
bool apply_quirk;
};
@@ -141,7 +143,7 @@ static struct rapl_pmus *rapl_pmus;
static cpumask_t rapl_cpu_mask;
static unsigned int rapl_cntr_mask;
static u64 rapl_timer_ms;
-static struct perf_msr rapl_msrs[];
+static struct perf_msr *rapl_msrs;
static inline struct rapl_pmu *cpu_to_rapl_pmu(unsigned int cpu)
{
@@ -516,7 +518,7 @@ static bool test_msr(int idx, void *data)
return test_bit(idx, (unsigned long *) data);
}
-static struct perf_msr rapl_msrs[] = {
+static struct perf_msr intel_rapl_msrs[] = {
[PERF_RAPL_PP0] = { MSR_PP0_ENERGY_STATUS, &rapl_events_cores_group, test_msr },
[PERF_RAPL_PKG] = { MSR_PKG_ENERGY_STATUS, &rapl_events_pkg_group, test_msr },
[PERF_RAPL_RAM] = { MSR_DRAM_ENERGY_STATUS, &rapl_events_ram_group, test_msr },
@@ -578,13 +580,13 @@ static int rapl_cpu_online(unsigned int cpu)
return 0;
}
-static int rapl_check_hw_unit(bool apply_quirk)
+static int rapl_check_hw_unit(struct rapl_model *rm)
{
u64 msr_rapl_power_unit_bits;
int i;
/* protect rdmsrl() to handle virtualization */
- if (rdmsrl_safe(MSR_RAPL_POWER_UNIT, &msr_rapl_power_unit_bits))
+ if (rdmsrl_safe(rm->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;
@@ -595,7 +597,7 @@ static int rapl_check_hw_unit(bool apply_quirk)
* "Intel Xeon Processor E5-1600 and E5-2600 v3 Product Families, V2
* of 2. Datasheet, September 2014, Reference Number: 330784-001 "
*/
- if (apply_quirk)
+ if (rm->apply_quirk)
rapl_hw_unit[PERF_RAPL_RAM] = 16;
/*
@@ -676,6 +678,8 @@ static struct rapl_model model_snb = {
BIT(PERF_RAPL_PKG) |
BIT(PERF_RAPL_PP1),
.apply_quirk = false,
+ .msr_power_unit = MSR_RAPL_POWER_UNIT,
+ .rapl_msrs = intel_rapl_msrs,
};
static struct rapl_model model_snbep = {
@@ -683,6 +687,8 @@ static struct rapl_model model_snbep = {
BIT(PERF_RAPL_PKG) |
BIT(PERF_RAPL_RAM),
.apply_quirk = false,
+ .msr_power_unit = MSR_RAPL_POWER_UNIT,
+ .rapl_msrs = intel_rapl_msrs,
};
static struct rapl_model model_hsw = {
@@ -691,6 +697,8 @@ static struct rapl_model model_hsw = {
BIT(PERF_RAPL_RAM) |
BIT(PERF_RAPL_PP1),
.apply_quirk = false,
+ .msr_power_unit = MSR_RAPL_POWER_UNIT,
+ .rapl_msrs = intel_rapl_msrs,
};
static struct rapl_model model_hsx = {
@@ -698,12 +706,16 @@ static struct rapl_model model_hsx = {
BIT(PERF_RAPL_PKG) |
BIT(PERF_RAPL_RAM),
.apply_quirk = true,
+ .msr_power_unit = MSR_RAPL_POWER_UNIT,
+ .rapl_msrs = intel_rapl_msrs,
};
static struct rapl_model model_knl = {
.events = BIT(PERF_RAPL_PKG) |
BIT(PERF_RAPL_RAM),
.apply_quirk = true,
+ .msr_power_unit = MSR_RAPL_POWER_UNIT,
+ .rapl_msrs = intel_rapl_msrs,
};
static struct rapl_model model_skl = {
@@ -713,6 +725,8 @@ static struct rapl_model model_skl = {
BIT(PERF_RAPL_PP1) |
BIT(PERF_RAPL_PSYS),
.apply_quirk = false,
+ .msr_power_unit = MSR_RAPL_POWER_UNIT,
+ .rapl_msrs = intel_rapl_msrs,
};
static const struct x86_cpu_id rapl_model_match[] __initconst = {
@@ -758,10 +772,13 @@ static int __init rapl_pmu_init(void)
return -ENODEV;
rm = (struct rapl_model *) id->driver_data;
+
+ rapl_msrs = rm->rapl_msrs;
+
rapl_cntr_mask = perf_msr_probe(rapl_msrs, PERF_RAPL_MAX,
false, (void *) &rm->events);
- ret = rapl_check_hw_unit(rm->apply_quirk);
+ ret = rapl_check_hw_unit(rm);
if (ret)
return ret;
--
2.26.2.761.g0e0b3e54be-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 2/3] perf/x86/rapl: refactor code for Intel/AMD sharing
2020-05-15 21:57 ` [PATCH 2/3] perf/x86/rapl: refactor code for Intel/AMD sharing Stephane Eranian
@ 2020-05-18 9:23 ` Peter Zijlstra
0 siblings, 0 replies; 11+ messages in thread
From: Peter Zijlstra @ 2020-05-18 9:23 UTC (permalink / raw)
To: Stephane Eranian; +Cc: linux-kernel, mingo, irogers, kim.phillips, jolsa
On Fri, May 15, 2020 at 02:57:32PM -0700, Stephane Eranian wrote:
> This patch modifies the rapl_model struct to include architecture specific
> knowledge to Intel specific structure, and in particular the MSR for
> POWER_UNIT and the rapl_msrs array.
>
> No functional changes.
>
> Signed-off-by: Stephane Eranian <eranian@google.com>
> ---
> arch/x86/events/rapl.c | 29 +++++++++++++++++++++++------
> 1 file changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/events/rapl.c b/arch/x86/events/rapl.c
> index ece043fb7b494..e98f627a13fa8 100644
> --- a/arch/x86/events/rapl.c
> +++ b/arch/x86/events/rapl.c
> @@ -131,7 +131,9 @@ struct rapl_pmus {
> };
>
> struct rapl_model {
> + struct perf_msr *rapl_msrs;
> unsigned long events;
> + int msr_power_unit;
MSR addresses go negative these days?
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/3] perf/x86/rapl: add AMD Fam17h RAPL support
2020-05-15 21:57 [PATCH 0/3] perf/x86/rapl: Enable RAPL for AMD Fam17h Stephane Eranian
2020-05-15 21:57 ` [PATCH 1/3] perf/x86/rapl: move RAPL support to common x86 code Stephane Eranian
2020-05-15 21:57 ` [PATCH 2/3] perf/x86/rapl: refactor code for Intel/AMD sharing Stephane Eranian
@ 2020-05-15 21:57 ` Stephane Eranian
2020-05-18 9:34 ` Peter Zijlstra
2020-05-16 16:56 ` [PATCH 0/3] perf/x86/rapl: Enable RAPL for AMD Fam17h Alexander Monakov
2020-05-18 8:50 ` Jiri Olsa
4 siblings, 1 reply; 11+ messages in thread
From: Stephane Eranian @ 2020-05-15 21:57 UTC (permalink / raw)
To: linux-kernel; +Cc: peterz, mingo, irogers, kim.phillips, jolsa
This patch enables AMD Fam17h RAPL support for the Package level metric.
The support is as per AMD Fam17h Model31h (Zen2) and model 00-ffh (Zen1) PPR.
The same output is available via the energy-pkg pseudo event:
$ perf stat -a -I 1000 --per-socket -e power/energy-pkg/
Signed-off-by: Stephane Eranian <eranian@google.com>
---
arch/x86/events/probe.c | 4 ++++
arch/x86/events/rapl.c | 17 +++++++++++++++++
arch/x86/include/asm/msr-index.h | 3 +++
3 files changed, 24 insertions(+)
diff --git a/arch/x86/events/probe.c b/arch/x86/events/probe.c
index c2ede2f3b2770..b3a9df2e48dfa 100644
--- a/arch/x86/events/probe.c
+++ b/arch/x86/events/probe.c
@@ -26,6 +26,10 @@ perf_msr_probe(struct perf_msr *msr, int cnt, bool zero, void *data)
grp->is_visible = not_visible;
+ /* avoid unpopulated entries */
+ if (!msr[bit].msr)
+ continue;
+
if (msr[bit].test && !msr[bit].test(bit, data))
continue;
/* Virt sucks; you cannot tell if a R/O MSR is present :/ */
diff --git a/arch/x86/events/rapl.c b/arch/x86/events/rapl.c
index e98f627a13fa8..47ff20dfde889 100644
--- a/arch/x86/events/rapl.c
+++ b/arch/x86/events/rapl.c
@@ -526,6 +526,15 @@ static struct perf_msr intel_rapl_msrs[] = {
[PERF_RAPL_PSYS] = { MSR_PLATFORM_ENERGY_STATUS, &rapl_events_psys_group, test_msr },
};
+static struct perf_msr amd_rapl_msrs[] = {
+ [PERF_RAPL_PP0] = { 0, &rapl_events_cores_group, NULL},
+ [PERF_RAPL_PKG] = { MSR_AMD_PKG_ENERGY_STATUS, &rapl_events_pkg_group, test_msr },
+ [PERF_RAPL_RAM] = { 0, &rapl_events_ram_group, NULL},
+ [PERF_RAPL_PP1] = { 0, &rapl_events_gpu_group, NULL},
+ [PERF_RAPL_PSYS] = { 0, &rapl_events_psys_group, NULL},
+};
+
+
static int rapl_cpu_offline(unsigned int cpu)
{
struct rapl_pmu *pmu = cpu_to_rapl_pmu(cpu);
@@ -729,6 +738,13 @@ static struct rapl_model model_skl = {
.rapl_msrs = intel_rapl_msrs,
};
+static struct rapl_model model_amd_fam17h = {
+ .events = BIT(PERF_RAPL_PKG),
+ .apply_quirk = false,
+ .msr_power_unit = MSR_AMD_RAPL_POWER_UNIT,
+ .rapl_msrs = amd_rapl_msrs,
+};
+
static const struct x86_cpu_id rapl_model_match[] __initconst = {
X86_MATCH_INTEL_FAM6_MODEL(SANDYBRIDGE, &model_snb),
X86_MATCH_INTEL_FAM6_MODEL(SANDYBRIDGE_X, &model_snbep),
@@ -757,6 +773,7 @@ static const struct x86_cpu_id rapl_model_match[] __initconst = {
X86_MATCH_INTEL_FAM6_MODEL(ICELAKE, &model_skl),
X86_MATCH_INTEL_FAM6_MODEL(COMETLAKE_L, &model_skl),
X86_MATCH_INTEL_FAM6_MODEL(COMETLAKE, &model_skl),
+ X86_MATCH_VENDOR_FAM(AMD, 0x17, &model_amd_fam17h),
{},
};
MODULE_DEVICE_TABLE(x86cpu, rapl_model_match);
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 12c9684d59ba6..ef452b817f44f 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -301,6 +301,9 @@
#define MSR_PP1_ENERGY_STATUS 0x00000641
#define MSR_PP1_POLICY 0x00000642
+#define MSR_AMD_PKG_ENERGY_STATUS 0xc001029b
+#define MSR_AMD_RAPL_POWER_UNIT 0xc0010299
+
/* Config TDP MSRs */
#define MSR_CONFIG_TDP_NOMINAL 0x00000648
#define MSR_CONFIG_TDP_LEVEL_1 0x00000649
--
2.26.2.761.g0e0b3e54be-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 3/3] perf/x86/rapl: add AMD Fam17h RAPL support
2020-05-15 21:57 ` [PATCH 3/3] perf/x86/rapl: add AMD Fam17h RAPL support Stephane Eranian
@ 2020-05-18 9:34 ` Peter Zijlstra
2020-05-18 20:16 ` Stephane Eranian
0 siblings, 1 reply; 11+ messages in thread
From: Peter Zijlstra @ 2020-05-18 9:34 UTC (permalink / raw)
To: Stephane Eranian; +Cc: linux-kernel, mingo, irogers, kim.phillips, jolsa
On Fri, May 15, 2020 at 02:57:33PM -0700, Stephane Eranian wrote:
> +static struct perf_msr amd_rapl_msrs[] = {
> + [PERF_RAPL_PP0] = { 0, &rapl_events_cores_group, NULL},
> + [PERF_RAPL_PKG] = { MSR_AMD_PKG_ENERGY_STATUS, &rapl_events_pkg_group, test_msr },
> + [PERF_RAPL_RAM] = { 0, &rapl_events_ram_group, NULL},
> + [PERF_RAPL_PP1] = { 0, &rapl_events_gpu_group, NULL},
> + [PERF_RAPL_PSYS] = { 0, &rapl_events_psys_group, NULL},
> +};
Why have those !PKG things initialized? Wouldn't they default to 0
anyway? If not, surely { 0, } is sufficient.
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 3/3] perf/x86/rapl: add AMD Fam17h RAPL support
2020-05-18 9:34 ` Peter Zijlstra
@ 2020-05-18 20:16 ` Stephane Eranian
2020-05-20 8:34 ` Stephane Eranian
0 siblings, 1 reply; 11+ messages in thread
From: Stephane Eranian @ 2020-05-18 20:16 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: LKML, mingo, Ian Rogers, Phillips, Kim, Jiri Olsa
On Mon, May 18, 2020 at 2:34 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Fri, May 15, 2020 at 02:57:33PM -0700, Stephane Eranian wrote:
>
> > +static struct perf_msr amd_rapl_msrs[] = {
> > + [PERF_RAPL_PP0] = { 0, &rapl_events_cores_group, NULL},
> > + [PERF_RAPL_PKG] = { MSR_AMD_PKG_ENERGY_STATUS, &rapl_events_pkg_group, test_msr },
> > + [PERF_RAPL_RAM] = { 0, &rapl_events_ram_group, NULL},
> > + [PERF_RAPL_PP1] = { 0, &rapl_events_gpu_group, NULL},
> > + [PERF_RAPL_PSYS] = { 0, &rapl_events_psys_group, NULL},
> > +};
>
> Why have those !PKG things initialized? Wouldn't they default to 0
> anyway? If not, surely { 0, } is sufficient.
Yes, but that assumes that perf_msr_probe() is fixed to not expect a grp.
I think it is best to fix perf_msr_probe(). I already fixed one
problem, I'll fix this one as well.
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 3/3] perf/x86/rapl: add AMD Fam17h RAPL support
2020-05-18 20:16 ` Stephane Eranian
@ 2020-05-20 8:34 ` Stephane Eranian
0 siblings, 0 replies; 11+ messages in thread
From: Stephane Eranian @ 2020-05-20 8:34 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: LKML, mingo, Ian Rogers, Phillips, Kim, Jiri Olsa
Hi,
On Mon, May 18, 2020 at 1:16 PM Stephane Eranian <eranian@google.com> wrote:
>
> On Mon, May 18, 2020 at 2:34 AM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > On Fri, May 15, 2020 at 02:57:33PM -0700, Stephane Eranian wrote:
> >
> > > +static struct perf_msr amd_rapl_msrs[] = {
> > > + [PERF_RAPL_PP0] = { 0, &rapl_events_cores_group, NULL},
> > > + [PERF_RAPL_PKG] = { MSR_AMD_PKG_ENERGY_STATUS, &rapl_events_pkg_group, test_msr },
> > > + [PERF_RAPL_RAM] = { 0, &rapl_events_ram_group, NULL},
> > > + [PERF_RAPL_PP1] = { 0, &rapl_events_gpu_group, NULL},
> > > + [PERF_RAPL_PSYS] = { 0, &rapl_events_psys_group, NULL},
> > > +};
> >
> > Why have those !PKG things initialized? Wouldn't they default to 0
> > anyway? If not, surely { 0, } is sufficient.
>
> Yes, but that assumes that perf_msr_probe() is fixed to not expect a grp.
> I think it is best to fix perf_msr_probe(). I already fixed one
> problem, I'll fix this one as well.
Well, now I remember what I did it the way it is in the patch. This
grp is going to sysfs, i.e., visible vs. not_visible callback.
Even if I fix the handling of NULL grp in perf_msr_probe(), the rest
of the rapl code pushes every event to sysfs and if the visible
callback is set to NULL this means the event is visible for sysfs! We
can fix that in init_rapl_pmus() but that is not pretty or leave it
as is, your call.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/3] perf/x86/rapl: Enable RAPL for AMD Fam17h
2020-05-15 21:57 [PATCH 0/3] perf/x86/rapl: Enable RAPL for AMD Fam17h Stephane Eranian
` (2 preceding siblings ...)
2020-05-15 21:57 ` [PATCH 3/3] perf/x86/rapl: add AMD Fam17h RAPL support Stephane Eranian
@ 2020-05-16 16:56 ` Alexander Monakov
2020-05-18 8:50 ` Jiri Olsa
4 siblings, 0 replies; 11+ messages in thread
From: Alexander Monakov @ 2020-05-16 16:56 UTC (permalink / raw)
To: Stephane Eranian
Cc: linux-kernel, peterz, mingo, irogers, kim.phillips, jolsa
On Fri, 15 May 2020, Stephane Eranian wrote:
> The series first moves the rapl.c file to common perf_events x86 and then
> adds the support.
> From the user's point of view, the interface is identical with
> /sys/devices/power. The energy-pkg event is the only one supported.
AMD also has per-core energy metering via MSR 0xc001029a, and I wonder
if you have plans to expose it to perf as well. I see it does not fit
so nicely with the existing code (as it's per-core instead of per-die).
The turbostat tool already exposes these per-core readings:
Core CPU Avg_MHz Busy% Bzy_MHz TSC_MHz CorWatt PkgWatt
- - 3951 100.00 3951 2373 54.92 30.04
0 0 3945 100.00 3945 2370 8.97 29.98
1 1 3945 100.00 3945 2370 9.11
2 2 3945 100.00 3945 2370 8.96
4 3 3946 100.00 3946 2370 9.32
5 4 3946 100.00 3946 2370 9.11
6 5 3946 100.00 3946 2370 9.39
turbostat sums the per-core energy figures to show the per-socket 54.92W
value. Though as you can see on this example, the figure is not in agreement
with the per-socket MSR you're using in your patch. Maybe the per-core
values are less reliable, but I believe I have a test that shows per-package
figure to be inaccurate as well. It would be nice if AMD clarified how the
counters work.
And, for what (little) it's worth, the series is:
Tested-by: Alexander Monakov <amonakov@ispras.ru>
Thank you.
Alexander
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 0/3] perf/x86/rapl: Enable RAPL for AMD Fam17h
2020-05-15 21:57 [PATCH 0/3] perf/x86/rapl: Enable RAPL for AMD Fam17h Stephane Eranian
` (3 preceding siblings ...)
2020-05-16 16:56 ` [PATCH 0/3] perf/x86/rapl: Enable RAPL for AMD Fam17h Alexander Monakov
@ 2020-05-18 8:50 ` Jiri Olsa
4 siblings, 0 replies; 11+ messages in thread
From: Jiri Olsa @ 2020-05-18 8:50 UTC (permalink / raw)
To: Stephane Eranian; +Cc: linux-kernel, peterz, mingo, irogers, kim.phillips
On Fri, May 15, 2020 at 02:57:30PM -0700, Stephane Eranian wrote:
> This patch series adds support for AMD Fam17h RAPL counters. As per
> AMD PPR, Fam17h support Package RAPL counters to monitor power usage.
> The RAPL counter operates as with Intel RAPL. As such, it is beneficial
> to share the code.
>
> The series first moves the rapl.c file to common perf_events x86 and then
> adds the support.
> From the user's point of view, the interface is identical with
> /sys/devices/power. The energy-pkg event is the only one supported.
>
> $ perf stat -a --per-socket -I 1000 -e power/energy-pkg/
>
> Signed-off-by: Stephane Eranian <eranian@google.com>
>
> Stephane Eranian (3):
> perf/x86/rapl: move RAPL support to common x86 code
> perf/x86/rapl: refactor code for Intel/AMD sharing
> perf/x86/rapl: add AMD Fam17h RAPL support
>
> arch/x86/events/Kconfig | 8 ++---
> arch/x86/events/Makefile | 1 +
> arch/x86/events/intel/Makefile | 2 --
> arch/x86/events/probe.c | 4 +++
> arch/x86/events/{intel => }/rapl.c | 55 +++++++++++++++++++++++++-----
> arch/x86/include/asm/msr-index.h | 3 ++
> 6 files changed, 58 insertions(+), 15 deletions(-)
> rename arch/x86/events/{intel => }/rapl.c (92%)
looks good to me, intel rapl keeps working for me ;-)
Acked-by: Jiri Olsa <jolsa@redhat.com>
thanks,
jirka
^ permalink raw reply [flat|nested] 11+ messages in thread