Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1361796

[PATCH 3/3] cpuidle, Prevent users from enabling cstates that are disabled in Hardware

From Prarit Bhargava <prarit@redhat.com>
Newsgroups linux.kernel
Subject [PATCH 3/3] cpuidle, Prevent users from enabling cstates that are disabled in Hardware
Date 2016-03-21 13:50 +0100
Message-ID <rf7rs-5J5-9@gated-at.bofh.it> (permalink)
References <rf7rs-5J5-11@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


The current codebase allows a user to enable a cstate even though the
hardware has disabled it.  This patch adds state checking and prevents
userspace from setting a hardware disabled cstate to enabled.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
---
 drivers/cpuidle/governors/ladder.c |    3 +++
 drivers/cpuidle/governors/menu.c   |    4 ++++
 drivers/cpuidle/sysfs.c            |   34 ++++++++++++++++++++++++++++++++--
 drivers/idle/intel_idle.c          |    6 ++++--
 include/linux/cpuidle.h            |   10 ++++++++--
 5 files changed, 51 insertions(+), 6 deletions(-)

diff --git a/drivers/cpuidle/governors/ladder.c b/drivers/cpuidle/governors/ladder.c
index 63bd5a4..680e5ee 100644
--- a/drivers/cpuidle/governors/ladder.c
+++ b/drivers/cpuidle/governors/ladder.c
@@ -144,6 +144,9 @@ static int ladder_enable_device(struct cpuidle_driver *drv,
 		state = &drv->states[i];
 		lstate = &ldev->states[i];
 
+		/* Some states may be disabled by hardware */
+		dev->states_usage[i].disable = drv->states[i].disabled;
+
 		lstate->stats.promotion_count = 0;
 		lstate->stats.demotion_count = 0;
 
diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c
index 0742b32..7351e0d 100644
--- a/drivers/cpuidle/governors/menu.c
+++ b/drivers/cpuidle/governors/menu.c
@@ -466,6 +466,10 @@ static int menu_enable_device(struct cpuidle_driver *drv,
 	for(i = 0; i < BUCKETS; i++)
 		data->correction_factor[i] = RESOLUTION * DECAY;
 
+	/* Some states may be disabled by hardware */
+	for (i = CPUIDLE_DRIVER_STATE_START; i < drv->state_count; i++)
+		dev->states_usage[i].disable = drv->states[i].disabled;
+
 	return 0;
 }
 
diff --git a/drivers/cpuidle/sysfs.c b/drivers/cpuidle/sysfs.c
index 832a2c3..313d1aa 100644
--- a/drivers/cpuidle/sysfs.c
+++ b/drivers/cpuidle/sysfs.c
@@ -274,6 +274,38 @@ static ssize_t store_state_##_name(struct cpuidle_state *state, \
 	return size; \
 }
 
+static ssize_t store_state_disable(struct cpuidle_state *state,
+				   struct cpuidle_state_usage *state_usage,
+				   const char *buf, size_t size)
+{
+	unsigned long long value;
+	int err;
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	if (state_usage->disable == CPUIDLE_STATE_HW_DISABLE) {
+		pr_info("cpuidle: state %s is disabled by hardware.\n",
+			state->name);
+		return -EINVAL;
+	}
+
+	err = kstrtoull(buf, 0, &value);
+	if (err)
+		return err;
+	if (value)
+		state_usage->disable = CPUIDLE_STATE_DISABLE;
+	else
+		state_usage->disable = CPUIDLE_STATE_ENABLE;
+	return size;
+}
+
+static ssize_t show_state_disable(struct cpuidle_state *state,
+				  struct cpuidle_state_usage *state_usage,
+				  char *buf)
+{
+	return sprintf(buf, "%u\n", !!state_usage->disable);
+}
+
 #define define_show_state_ull_function(_name) \
 static ssize_t show_state_##_name(struct cpuidle_state *state, \
 				  struct cpuidle_state_usage *state_usage, \
@@ -299,8 +331,6 @@ define_show_state_ull_function(usage)
 define_show_state_ull_function(time)
 define_show_state_str_function(name)
 define_show_state_str_function(desc)
-define_show_state_ull_function(disable)
-define_store_state_ull_function(disable)
 
 define_one_state_ro(name, show_state_name);
 define_one_state_ro(desc, show_state_desc);
diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index 58bc913..f23d0f6 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -1142,10 +1142,12 @@ static int __init intel_idle_cpuidle_driver_init(void)
 
 		/* Has this state been disabled in hardware? */
 		if (limit < cpuidle_state_table[cstate].limit) {
-			cpuidle_state_table[cstate].disabled = 1;
-			pr_debug(PREFIX "state %s (0x%x) is disabled.  Max Package limit is 0x%x.\n",
+			cpuidle_state_table[cstate].disabled =
+						       CPUIDLE_STATE_HW_DISABLE;
+			pr_debug(PREFIX "state %s (0x%x) is disabled and set to %d.  Max Package limit is 0x%x.\n",
 				 cpuidle_state_table[cstate].name,
 				 cpuidle_state_table[cstate].limit,
+				 cpuidle_state_table[cstate].disabled,
 				 limit);
 
 		}
diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
index 8bcfabb..0e59e17 100644
--- a/include/linux/cpuidle.h
+++ b/include/linux/cpuidle.h
@@ -29,8 +29,14 @@ struct cpuidle_driver;
  * CPUIDLE DEVICE INTERFACE *
  ****************************/
 
+enum cpuidle_state_disable {
+	CPUIDLE_STATE_ENABLE = 0,
+	CPUIDLE_STATE_DISABLE = 1,
+	CPUIDLE_STATE_HW_DISABLE = 2,
+};
+
 struct cpuidle_state_usage {
-	unsigned long long	disable;
+	enum cpuidle_state_disable disable;
 	unsigned long long	usage;
 	unsigned long long	time; /* in US */
 };
@@ -44,7 +50,7 @@ struct cpuidle_state {
 	unsigned int	exit_latency; /* in US */
 	int		power_usage; /* in mW */
 	unsigned int	target_residency; /* in US */
-	bool		disabled; /* disabled on all CPUs */
+	enum cpuidle_state_disable	disabled; /* disabled on all CPUs */
 
 	int (*enter)	(struct cpuidle_device *dev,
 			struct cpuidle_driver *drv,
-- 
1.7.9.3

Back to linux.kernel | Previous | Next | Find similar | Unroll thread


Thread

[PATCH 3/3] cpuidle, Prevent users from enabling cstates that are disabled in Hardware Prarit Bhargava <prarit@redhat.com> - 2016-03-21 13:50 +0100

csiph-web