Skip to contentSkip to conversation

: ACPI: Add the module param check for processor.max_cstate

PATCH In review yakui_zhao 2 participants 3 messages Apr 24, 2009 ACPI

lore
Share & tools
lore

yakui_zhao

author
#0 lore raw

PATCH 1/1 : ACPI: Add the module param check for processor.max_cstate

From: Zhao Yakui <[email protected]>

When the boot option of "processor.max_cstate=0" is added, the C-state won't be configured correctly. In such case the cpu idle function will access the uninitialized memory region and then kernel panic will happen.

So it is necessary to add the module param check for the processor.max_cstate. If it is equal to or less than zero, it will be ignored. If it is beyond the max threshold, it is also ignored.

http://bugzilla.kernel.org/show_bug.cgi?id=13142

---
 drivers/acpi/processor_idle.c |   27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)
drivers/acpi/processor_idle.c +26 -1
Index: linux-2.6/drivers/acpi/processor_idle.c
===================================================================
@@ -70,7 +70,32 @@
 #define PM_TIMER_TICKS_TO_US(p)		(((p) * 1000)/(PM_TIMER_FREQUENCY/1000))
 
 static unsigned int max_cstate __read_mostly = ACPI_PROCESSOR_MAX_POWER;
-module_param(max_cstate, uint, 0000);
+
+static int param_set_max_cstate(const char *val, struct kernel_param *kp)
+{
+	unsigned long cstate = 0;
+	int ret = 0;
+
+	if (!val)
+		return -EINVAL;
+
+	ret = strict_strtoul(val, 10, &cstate);
+	if (ret == -EINVAL || (unsigned int)cstate != cstate)
+		return -EINVAL;
+	/*
+	 * if the value is equal to or less than zero, it will be ingored.
+	 * Of course it will also be ignored if it is bigger than the
+	 * threshold of ACPI_PROCESSOR_MAX_POWER.
+	 */
+	if (cstate <= 0 || cstate > ACPI_PROCESSOR_MAX_POWER)
+		return -EINVAL;
+
+	max_cstate = cstate;
+
+	return 0;
+}
+module_param_call(max_cstate, param_set_max_cstate, NULL,
+		  &max_cstate, 0000);
 static unsigned int nocst __read_mostly;
 module_param(nocst, uint, 0000);
 
reviewer
#1 lore raw

From: Len Brown <[email protected]>
Subject: [PATCH] ACPI: prevent processor.max_cstate=0 boot crash

As processor.max_cstate is an init-time-only modparam, sanity checking it at init-time is sufficient.

http://bugzilla.kernel.org/show_bug.cgi?id=13142

---
 drivers/acpi/processor_idle.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
drivers/acpi/processor_idle.c +3 -0
index 6fe1214..436127e 100644
@@ -1037,6 +1037,9 @@ static int acpi_processor_setup_cpuidle(struct acpi_processor *pr)
 		dev->states[i].desc[0] = '\0';
 	}
 
+	if (max_cstate == 0)
+		max_cstate = 1;
+
 	for (i = 1; i < ACPI_PROCESSOR_MAX_POWER && i <= max_cstate; i++) {
 		cx = &pr->power.states[i];
 		state = &dev->states[count];
Signature · 1 line
1.6.3.rc1.34.g0be9b
author
#2 lore raw
Quoted 7 lines · Len Brown

From: Len Brown <[email protected]>
Subject: [PATCH] ACPI: prevent processor.max_cstate=0 boot crash

As processor.max_cstate is an init-time-only modparam, sanity checking it at init-time is sufficient.

http://bugzilla.kernel.org/show_bug.cgi?id=13142

This is also OK. And it is easier.

Thanks.

Quoted 20 lines
---
 drivers/acpi/processor_idle.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
drivers/acpi/processor_idle.c +3 -0
index 6fe1214..436127e 100644
@@ -1037,6 +1037,9 @@ static int acpi_processor_setup_cpuidle(struct acpi_processor *pr)
 		dev->states[i].desc[0] = '\0';
 	}
 
+	if (max_cstate == 0)
+		max_cstate = 1;
+
 	for (i = 1; i < ACPI_PROCESSOR_MAX_POWER && i <= max_cstate; i++) {
 		cx = &pr->power.states[i];
 		state = &dev->states[count];