Thread (9 messages) 9 messages, 4 authors, 2016-06-16

Re: [PATCH 1/2] powercap/rapl: handle missing msrs

From: jacob <hidden>
Date: 2016-06-13 18:53:41
Also in: lkml

Hi Rafael,

Any feedback? It that is OK, can you take this patch independent of the
second patch (which is going into tip tree)?
[PATCH 2/2] powercap/rapl: add support for denverton

This patch affects some Broxton/Apollo Lake where the missing MSR will
cause the driver fail to load.

On Tue, 31 May 2016 13:41:29 -0700
Jacob Pan [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Some RAPL MSRs may not exist on some CPUs, we need to continue
the topology detection and enumerate what is available.

This patch handles the missing MSRs then report to powercap
layer only the features available.

Signed-off-by: Jacob Pan <redacted>
---
 drivers/powercap/intel_rapl.c | 103
++++++++++++++++++++++++++++++++---------- 1 file changed, 79
insertions(+), 24 deletions(-)
diff --git a/drivers/powercap/intel_rapl.c
b/drivers/powercap/intel_rapl.c index b0a2dc4..d51a8d4 100644
--- a/drivers/powercap/intel_rapl.c
+++ b/drivers/powercap/intel_rapl.c
@@ -329,14 +329,14 @@ static int release_zone(struct powercap_zone
*power_zone) 
 static int find_nr_power_limit(struct rapl_domain *rd)
 {
-	int i;
+	int i, nr_pl = 0;
 
 	for (i = 0; i < NR_POWER_LIMITS; i++) {
-		if (rd->rpl[i].name == NULL)
-			break;
+		if (rd->rpl[i].name)
+			nr_pl++;
 	}
 
-	return i;
+	return nr_pl;
 }
 
 static int set_domain_enable(struct powercap_zone *power_zone, bool
mode) @@ -411,15 +411,38 @@ static const struct powercap_zone_ops
zone_ops[] = { },
 };
 
-static int set_power_limit(struct powercap_zone *power_zone, int id,
+
+/*
+ * Constraint index used by powercap can be different than power
limit (PL)
+ * index in that some  PLs maybe missing due to non-existant MSRs.
So we
+ * need to convert here by finding the valid PLs only (name
populated).
+ */
+static int contraint_to_pl(struct rapl_domain *rd, int cid)
+{
+	int i, j;
+
+	for (i = 0, j = 0; i < NR_POWER_LIMITS; i++) {
+		if ((rd->rpl[i].name) && j++ == cid) {
+			pr_debug("%s: index %d\n", __func__, i);
+			return i;
+		}
+	}
+
+	return -EINVAL;
+}
+
+static int set_power_limit(struct powercap_zone *power_zone, int cid,
 			u64 power_limit)
 {
 	struct rapl_domain *rd;
 	struct rapl_package *rp;
 	int ret = 0;
+	int id;
 
 	get_online_cpus();
 	rd = power_zone_to_rapl_domain(power_zone);
+	id = contraint_to_pl(rd, cid);
+
 	rp = rd->rp;
 
 	if (rd->state & DOMAIN_STATE_BIOS_LOCKED) {
@@ -446,16 +469,18 @@ set_exit:
 	return ret;
 }
 
-static int get_current_power_limit(struct powercap_zone *power_zone,
int id, +static int get_current_power_limit(struct powercap_zone
*power_zone, int cid, u64 *data)
 {
 	struct rapl_domain *rd;
 	u64 val;
 	int prim;
 	int ret = 0;
+	int id;
 
 	get_online_cpus();
 	rd = power_zone_to_rapl_domain(power_zone);
+	id = contraint_to_pl(rd, cid);
 	switch (rd->rpl[id].prim_id) {
 	case PL1_ENABLE:
 		prim = POWER_LIMIT1;
@@ -477,14 +502,17 @@ static int get_current_power_limit(struct
powercap_zone *power_zone, int id, return ret;
 }
 
-static int set_time_window(struct powercap_zone *power_zone, int id,
+static int set_time_window(struct powercap_zone *power_zone, int cid,
 								u64
window) {
 	struct rapl_domain *rd;
 	int ret = 0;
+	int id;
 
 	get_online_cpus();
 	rd = power_zone_to_rapl_domain(power_zone);
+	id = contraint_to_pl(rd, cid);
+
 	switch (rd->rpl[id].prim_id) {
 	case PL1_ENABLE:
 		rapl_write_data_raw(rd, TIME_WINDOW1, window);
@@ -499,14 +527,17 @@ static int set_time_window(struct powercap_zone
*power_zone, int id, return ret;
 }
 
-static int get_time_window(struct powercap_zone *power_zone, int id,
u64 *data) +static int get_time_window(struct powercap_zone
*power_zone, int cid, u64 *data) {
 	struct rapl_domain *rd;
 	u64 val;
 	int ret = 0;
+	int id;
 
 	get_online_cpus();
 	rd = power_zone_to_rapl_domain(power_zone);
+	id = contraint_to_pl(rd, cid);
+
 	switch (rd->rpl[id].prim_id) {
 	case PL1_ENABLE:
 		ret = rapl_read_data_raw(rd, TIME_WINDOW1, true,
&val); @@ -525,15 +556,17 @@ static int get_time_window(struct
powercap_zone *power_zone, int id, u64 *data) return ret;
 }
 
-static const char *get_constraint_name(struct powercap_zone
*power_zone, int id) +static const char *get_constraint_name(struct
powercap_zone *power_zone, int cid) {
-	struct rapl_power_limit *rpl;
 	struct rapl_domain *rd;
+	int id;
 
 	rd = power_zone_to_rapl_domain(power_zone);
-	rpl = (struct rapl_power_limit *) &rd->rpl[id];
+	id = contraint_to_pl(rd, cid);
+	if (id >= 0)
+		return rd->rpl[id].name;
 
-	return rpl->name;
+	return NULL;
 }
 
 
@@ -1305,6 +1338,37 @@ static int rapl_check_domain(int cpu, int
domain) return 0;
 }
 
+
+/*
+ * Check if power limits are available. Two cases when they are not
available:
+ * 1. Locked by BIOS, in this case we still provide read-only access
so that
+ *    users can see what limit is set by the BIOS.
+ * 2. Some CPUs make some domains monitoring only which means PLx
MSRs may not
+ *    exist at all. In this case, we do not show the contraints in
powercap.
+ *
+ * Called after domains are detected and initialized.
+ */
+static void rapl_detect_powerlimit(struct rapl_domain *rd)
+{
+	u64 val64;
+	int i;
+
+	/* check if the domain is locked by BIOS, ignore if MSR
doesn't exist */
+	if (!rapl_read_data_raw(rd, FW_LOCK, false, &val64)) {
+		if (val64) {
+			pr_info("RAPL package %d domain %s locked by
BIOS\n",
+				rd->rp->id, rd->name);
+			rd->state |= DOMAIN_STATE_BIOS_LOCKED;
+		}
+	}
+	/* check if power limit MSRs exists, otherwise domain is
monitoring only */
+	for (i = 0; i < NR_POWER_LIMITS; i++) {
+		int prim = rd->rpl[i].prim_id;
+		if (rapl_read_data_raw(rd, prim, false, &val64))
+			rd->rpl[i].name = NULL;
+	}
+}
+
 /* Detect active and valid domains for the given CPU, caller must
  * ensure the CPU belongs to the targeted package and CPU hotlug is
disabled. */
@@ -1313,7 +1377,6 @@ static int rapl_detect_domains(struct
rapl_package *rp, int cpu) int i;
 	int ret = 0;
 	struct rapl_domain *rd;
-	u64 locked;
 
 	for (i = 0; i < RAPL_DOMAIN_MAX; i++) {
 		/* use physical package id to read counters */
@@ -1338,17 +1401,9 @@ static int rapl_detect_domains(struct
rapl_package *rp, int cpu) }
 	rapl_init_domains(rp);
 
-	for (rd = rp->domains; rd < rp->domains + rp->nr_domains;
rd++) {
-		/* check if the domain is locked by BIOS */
-		ret = rapl_read_data_raw(rd, FW_LOCK, false,
&locked);
-		if (ret)
-			return ret;
-		if (locked) {
-			pr_info("RAPL package %d domain %s locked by
BIOS\n",
-				rp->id, rd->name);
-			rd->state |= DOMAIN_STATE_BIOS_LOCKED;
-		}
-	}
+	for (rd = rp->domains; rd < rp->domains + rp->nr_domains;
rd++)
+		rapl_detect_powerlimit(rd);
+
 
 
 done:
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help