[PATCH 0/3] powercap: Enable RAPL for AMD Fam17h

[PATCH 0/3] powercap: Enable RAPL for AMD Fam17h

[Victor Ding]

This patch series adds support for AMD Fam17h RAPL counters. As per
AMD PPR, Fam17h support RAPL counters to monitor power usage. The RAPL
counter operates as with Intel RAPL. Therefore, it is beneficial to
re-use existing framework for Intel, especially to allow existing tools
to seamlessly run on AMD.

From the user's point view, this series enables the following two sysfs
entry on AMD Fam17h:
  /sys/class/powercap/intel-rapl/intel-rapl:0/energy_uj
  /sys/class/powercap/intel-rapl/intel-rapl:0/intel-rapl:0:0/energy_uj


Victor Ding (3):
  x86/msr-index: sort AMD RAPL MSRs by address
  powercap/intel_rapl_msr: Convert rapl_msr_priv into pointer
  powercap: Add AMD Fam17h RAPL support

 arch/x86/include/asm/msr-index.h     |  3 +-
 drivers/powercap/intel_rapl_common.c |  2 +
 drivers/powercap/intel_rapl_msr.c    | 58 +++++++++++++++++++++-------
 3 files changed, 47 insertions(+), 16 deletions(-)

-- 
2.28.0.rc0.142.g3c755180ce-goog

[PATCH 1/3] x86/msr-index: sort AMD RAPL MSRs by address

[Victor Ding]

MSRs in the rest of this file are sorted by their addresses; fixing the
two outliers.

No functional changes.

Signed-off-by: Victor Ding <victording@google.com>
---

 arch/x86/include/asm/msr-index.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index e8370e64a155..c5e92317356e 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -305,8 +305,8 @@
 #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
+#define MSR_AMD_PKG_ENERGY_STATUS	0xc001029b
 
 /* Config TDP MSRs */
 #define MSR_CONFIG_TDP_NOMINAL		0x00000648
-- 
2.28.0.rc0.142.g3c755180ce-goog

[PATCH 2/3] powercap/intel_rapl_msr: Convert rapl_msr_priv into pointer

[Victor Ding]

This patch changes the static struct rapl_msr_priv to a pointer to allow
using a different set of of RAPL MSR interface, preparing for supporting
AMD's RAPL MSR interface.

No functional changes.

Signed-off-by: Victor Ding <victording@google.com>
---

 drivers/powercap/intel_rapl_msr.c | 33 +++++++++++++++++--------------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/drivers/powercap/intel_rapl_msr.c b/drivers/powercap/intel_rapl_msr.c
index d5487965bdfe..430b4819d6ab 100644
--- a/drivers/powercap/intel_rapl_msr.c
+++ b/drivers/powercap/intel_rapl_msr.c
@@ -30,7 +30,9 @@
 #define MSR_PLATFORM_POWER_LIMIT	0x0000065C
 
 /* private data for RAPL MSR Interface */
-static struct rapl_if_priv rapl_msr_priv = {
+static struct rapl_if_priv *rapl_msr_priv;
+
+static struct rapl_if_priv rapl_msr_priv_intel = {
 	.reg_unit = MSR_RAPL_POWER_UNIT,
 	.regs[RAPL_DOMAIN_PACKAGE] = {
 		MSR_PKG_POWER_LIMIT, MSR_PKG_ENERGY_STATUS, MSR_PKG_PERF_STATUS, 0, MSR_PKG_POWER_INFO },
@@ -56,9 +58,9 @@ static int rapl_cpu_online(unsigned int cpu)
 {
 	struct rapl_package *rp;
 
-	rp = rapl_find_package_domain(cpu, &rapl_msr_priv);
+	rp = rapl_find_package_domain(cpu, rapl_msr_priv);
 	if (!rp) {
-		rp = rapl_add_package(cpu, &rapl_msr_priv);
+		rp = rapl_add_package(cpu, rapl_msr_priv);
 		if (IS_ERR(rp))
 			return PTR_ERR(rp);
 	}
@@ -71,7 +73,7 @@ static int rapl_cpu_down_prep(unsigned int cpu)
 	struct rapl_package *rp;
 	int lead_cpu;
 
-	rp = rapl_find_package_domain(cpu, &rapl_msr_priv);
+	rp = rapl_find_package_domain(cpu, rapl_msr_priv);
 	if (!rp)
 		return 0;
 
@@ -127,37 +129,38 @@ static int rapl_msr_probe(struct platform_device *pdev)
 {
 	int ret;
 
-	rapl_msr_priv.read_raw = rapl_msr_read_raw;
-	rapl_msr_priv.write_raw = rapl_msr_write_raw;
+	rapl_msr_priv = &rapl_msr_priv_intel;
+	rapl_msr_priv->read_raw = rapl_msr_read_raw;
+	rapl_msr_priv->write_raw = rapl_msr_write_raw;
 
-	rapl_msr_priv.control_type = powercap_register_control_type(NULL, "intel-rapl", NULL);
-	if (IS_ERR(rapl_msr_priv.control_type)) {
+	rapl_msr_priv->control_type = powercap_register_control_type(NULL, "intel-rapl", NULL);
+	if (IS_ERR(rapl_msr_priv->control_type)) {
 		pr_debug("failed to register powercap control_type.\n");
-		return PTR_ERR(rapl_msr_priv.control_type);
+		return PTR_ERR(rapl_msr_priv->control_type);
 	}
 
 	ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "powercap/rapl:online",
 				rapl_cpu_online, rapl_cpu_down_prep);
 	if (ret < 0)
 		goto out;
-	rapl_msr_priv.pcap_rapl_online = ret;
+	rapl_msr_priv->pcap_rapl_online = ret;
 
 	/* Don't bail out if PSys is not supported */
-	rapl_add_platform_domain(&rapl_msr_priv);
+	rapl_add_platform_domain(rapl_msr_priv);
 
 	return 0;
 
 out:
 	if (ret)
-		powercap_unregister_control_type(rapl_msr_priv.control_type);
+		powercap_unregister_control_type(rapl_msr_priv->control_type);
 	return ret;
 }
 
 static int rapl_msr_remove(struct platform_device *pdev)
 {
-	cpuhp_remove_state(rapl_msr_priv.pcap_rapl_online);
-	rapl_remove_platform_domain(&rapl_msr_priv);
-	powercap_unregister_control_type(rapl_msr_priv.control_type);
+	cpuhp_remove_state(rapl_msr_priv->pcap_rapl_online);
+	rapl_remove_platform_domain(rapl_msr_priv);
+	powercap_unregister_control_type(rapl_msr_priv->control_type);
 	return 0;
 }
 
-- 
2.28.0.rc0.142.g3c755180ce-goog

[PATCH 3/3] powercap: Add AMD Fam17h RAPL support

[Victor Ding]

This patch enables AMD Fam17h RAPL support for the power capping
framework. The support is as per AMD Fam17h Model31h (Zen2) and
model 00-ffh (Zen1) PPR.

Tested by comparing the results of following two sysfs entries and the
values directly read from corresponding MSRs via /dev/cpu/[x]/msr:
  /sys/class/powercap/intel-rapl/intel-rapl:0/energy_uj
  /sys/class/powercap/intel-rapl/intel-rapl:0/intel-rapl:0:0/energy_uj

Signed-off-by: Victor Ding <victording@google.com>
---

 arch/x86/include/asm/msr-index.h     |  1 +
 drivers/powercap/intel_rapl_common.c |  2 ++
 drivers/powercap/intel_rapl_msr.c    | 27 ++++++++++++++++++++++++++-
 3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index c5e92317356e..533208998297 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -306,6 +306,7 @@
 #define MSR_PP1_POLICY			0x00000642
 
 #define MSR_AMD_RAPL_POWER_UNIT		0xc0010299
+#define MSR_AMD_CORE_ENERGY_STATUS	0xc001029a
 #define MSR_AMD_PKG_ENERGY_STATUS	0xc001029b
 
 /* Config TDP MSRs */
diff --git a/drivers/powercap/intel_rapl_common.c b/drivers/powercap/intel_rapl_common.c
index 61a63a16b5e7..8ca4413ef2de 100644
--- a/drivers/powercap/intel_rapl_common.c
+++ b/drivers/powercap/intel_rapl_common.c
@@ -992,6 +992,8 @@ static const struct x86_cpu_id rapl_ids[] __initconst = {
 
 	X86_MATCH_INTEL_FAM6_MODEL(XEON_PHI_KNL,	&rapl_defaults_hsw_server),
 	X86_MATCH_INTEL_FAM6_MODEL(XEON_PHI_KNM,	&rapl_defaults_hsw_server),
+
+	X86_MATCH_VENDOR_FAM(AMD, 0x17, &rapl_defaults_core),
 	{}
 };
 MODULE_DEVICE_TABLE(x86cpu, rapl_ids);
diff --git a/drivers/powercap/intel_rapl_msr.c b/drivers/powercap/intel_rapl_msr.c
index 430b4819d6ab..31d10a169e41 100644
--- a/drivers/powercap/intel_rapl_msr.c
+++ b/drivers/powercap/intel_rapl_msr.c
@@ -47,6 +47,21 @@ static struct rapl_if_priv rapl_msr_priv_intel = {
 	.limits[RAPL_DOMAIN_PACKAGE] = 2,
 };
 
+static struct rapl_if_priv rapl_msr_priv_amd = {
+	.reg_unit = MSR_AMD_RAPL_POWER_UNIT,
+	.regs[RAPL_DOMAIN_PACKAGE] = {
+		0, MSR_AMD_PKG_ENERGY_STATUS, 0, 0, 0 },
+	.regs[RAPL_DOMAIN_PP0] = {
+		0, MSR_AMD_CORE_ENERGY_STATUS, 0, 0, 0 },
+	.regs[RAPL_DOMAIN_PP1] = {
+		0, 0, 0, 0, 0 },
+	.regs[RAPL_DOMAIN_DRAM] = {
+		0, 0, 0, 0, 0 },
+	.regs[RAPL_DOMAIN_PLATFORM] = {
+		0, 0, 0, 0, 0},
+	.limits[RAPL_DOMAIN_PACKAGE] = 1,
+};
+
 /* Handles CPU hotplug on multi-socket systems.
  * If a CPU goes online as the first CPU of the physical package
  * we add the RAPL package to the system. Similarly, when the last
@@ -129,7 +144,17 @@ static int rapl_msr_probe(struct platform_device *pdev)
 {
 	int ret;
 
-	rapl_msr_priv = &rapl_msr_priv_intel;
+	switch (boot_cpu_data.x86_vendor) {
+	case X86_VENDOR_INTEL:
+		rapl_msr_priv = &rapl_msr_priv_intel;
+		break;
+	case X86_VENDOR_AMD:
+		rapl_msr_priv = &rapl_msr_priv_amd;
+		break;
+	default:
+		pr_err("intel-rapl does not support CPU vendor %d\n", boot_cpu_data.x86_vendor);
+		return -ENODEV;
+	}
 	rapl_msr_priv->read_raw = rapl_msr_read_raw;
 	rapl_msr_priv->write_raw = rapl_msr_write_raw;
 
-- 
2.28.0.rc0.142.g3c755180ce-goog

Re: [PATCH 3/3] powercap: Add AMD Fam17h RAPL support

[Kim Phillips]

On 7/29/20 5:52 AM, Victor Ding wrote:
> This patch enables AMD Fam17h RAPL support for the power capping
> framework. The support is as per AMD Fam17h Model31h (Zen2) and
> model 00-ffh (Zen1) PPR.
> 
> Tested by comparing the results of following two sysfs entries and the
> values directly read from corresponding MSRs via /dev/cpu/[x]/msr:
>   /sys/class/powercap/intel-rapl/intel-rapl:0/energy_uj
>   /sys/class/powercap/intel-rapl/intel-rapl:0/intel-rapl:0:0/energy_uj
> 
> Signed-off-by: Victor Ding <victording@google.com>
> ---

For the series:

Acked-by: Kim Phillips <kim.phillips@amd.com>

Thanks,

Kim

Re: [PATCH 0/3] powercap: Enable RAPL for AMD Fam17h

[Rafael J. Wysocki]

On Wed, Jul 29, 2020 at 12:52 PM Victor Ding <victording@google.com> wrote:
>
> This patch series adds support for AMD Fam17h RAPL counters. As per
> AMD PPR, Fam17h support RAPL counters to monitor power usage. The RAPL
> counter operates as with Intel RAPL. Therefore, it is beneficial to
> re-use existing framework for Intel, especially to allow existing tools
> to seamlessly run on AMD.
>
> From the user's point view, this series enables the following two sysfs
> entry on AMD Fam17h:
>   /sys/class/powercap/intel-rapl/intel-rapl:0/energy_uj
>   /sys/class/powercap/intel-rapl/intel-rapl:0/intel-rapl:0:0/energy_uj
>
>
> Victor Ding (3):
>   x86/msr-index: sort AMD RAPL MSRs by address

I haven't received this patch.

Any chance to resend the entire series with a CC to linux-pm?

>   powercap/intel_rapl_msr: Convert rapl_msr_priv into pointer
>   powercap: Add AMD Fam17h RAPL support
>
>  arch/x86/include/asm/msr-index.h     |  3 +-
>  drivers/powercap/intel_rapl_common.c |  2 +
>  drivers/powercap/intel_rapl_msr.c    | 58 +++++++++++++++++++++-------
>  3 files changed, 47 insertions(+), 16 deletions(-)
>
> --
> 2.28.0.rc0.142.g3c755180ce-goog
>

Re: [PATCH 0/3] powercap: Enable RAPL for AMD Fam17h

[Daniel Lezcano]

On 21/09/2020 17:51, Rafael J. Wysocki wrote:
> On Wed, Jul 29, 2020 at 12:52 PM Victor Ding <victording@google.com> wrote:
>>
>> This patch series adds support for AMD Fam17h RAPL counters. As per
>> AMD PPR, Fam17h support RAPL counters to monitor power usage. The RAPL
>> counter operates as with Intel RAPL. Therefore, it is beneficial to
>> re-use existing framework for Intel, especially to allow existing tools
>> to seamlessly run on AMD.
>>
>> From the user's point view, this series enables the following two sysfs
>> entry on AMD Fam17h:
>>   /sys/class/powercap/intel-rapl/intel-rapl:0/energy_uj
>>   /sys/class/powercap/intel-rapl/intel-rapl:0/intel-rapl:0:0/energy_uj
>>
>>
>> Victor Ding (3):
>>   x86/msr-index: sort AMD RAPL MSRs by address
> 
> I haven't received this patch.
> 
> Any chance to resend the entire series with a CC to linux-pm?

Is it possible to add me in Cc too ?

Thanks




-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog