[patch 1/3] acpi: check _PSS invalidation when BIOS report _PSS with 0x80000000

[patch 1/3] acpi: check _PSS invalidation when BIOS report _PSS with 0x80000000

[akpm]

From: Youquan, Song <youquan.song@intel.com>

When cpu frequencey scaling is disabled, some BIOSes report _PSS with all
0x80000000.  If the kernel treats this case as valid, the kernel will boot
crash when load cpufreq govenors.

So in order to cover more buggy BIOSs, the patch just check _PSS core
frequency invalidation.

On Sun, 15 Mar 2009 23:22:37 -0400 (EDT) Len Brown <lenb@kernel.org> wrote:

> NAK -- per previous discussion on the list.
> 
> This patch is not the right way to address this issue,
> which is reported here, BTW:
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=467941

Signed-off-by: Youquan, Song <youquan.song@intel.com>
Signed-off-by: Pallipadi, Venkatesh <venkatesh.pallipadi@intel.com>
Cc: Len Brown <lenb@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 drivers/acpi/processor_perflib.c |    9 +++++++++
 1 file changed, 9 insertions(+)

diff -puN drivers/acpi/processor_perflib.c~acpi-check-_pss-invalidation-when-bios-report-_pss-with-all-0x80000000 drivers/acpi/processor_perflib.c
--- a/drivers/acpi/processor_perflib.c~acpi-check-_pss-invalidation-when-bios-report-_pss-with-all-0x80000000
+++ a/drivers/acpi/processor_perflib.c
@@ -42,6 +42,7 @@
 #define ACPI_PROCESSOR_CLASS		"processor"
 #define ACPI_PROCESSOR_FILE_PERFORMANCE	"performance"
 #define _COMPONENT		ACPI_PROCESSOR_COMPONENT
+#define ACPI_PROCESSOR_PSS_INVALID	0x80000000
 ACPI_MODULE_NAME("processor_perflib");
 
 static DEFINE_MUTEX(performance_mutex);
@@ -316,6 +317,14 @@ static int acpi_processor_get_performanc
 			kfree(pr->performance->states);
 			goto end;
 		}
+
+		if (px->core_frequency == ACPI_PROCESSOR_PSS_INVALID) {
+			printk(KERN_ERR PREFIX
+				    "P-states disabled in the BIOS\n");
+			result = -EFAULT;
+			kfree(pr->performance->states);
+			goto end;
+		}
 	}
 
       end:
_

Re: [patch 1/3] acpi: check _PSS invalidation when BIOS report _PSS with 0x80000000

[Len Brown]

NAK

This is a recording.

Andrew, please remove this patch from your series
per issues earlier discussed on the list.

thanks,
Len Brown, Intel Open Source Technology Center

Re: [patch 1/3] acpi: check _PSS invalidation when BIOS report _PSS with 0x80000000

[Andrew Morton]

On Thu, 14 May 2009 13:35:10 -0400 (EDT)
Len Brown <lenb@kernel.org> wrote:

> NAK
> 
> This is a recording.
> 
> Andrew, please remove this patch from your series
> per issues earlier discussed on the list.
> 


Did the problem get fixed?

Re: [patch 1/3] acpi: check _PSS invalidation when BIOS report _PSS with 0x80000000

[Matthew Garrett]

On Thu, May 14, 2009 at 01:35:10PM -0400, Len Brown wrote:
> NAK
> 
> This is a recording.
> 
> Andrew, please remove this patch from your series
> per issues earlier discussed on the list.

On the dump I have from a machine showing this issue, we'd fail 
gracefully if we checked the return value of 
acpi_processor_preregister_performance() as the coordination type is set 
to an invalid value. Are there any cases where this fails and the 
hardware still works? If not, then:

diff --git a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
index 208ecf6..fbb8e59 100644
--- a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
+++ b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
@@ -561,7 +561,11 @@ static int __init acpi_cpufreq_early_init(void)
 	}
 
 	/* Do initialization in ACPI core */
-	acpi_processor_preregister_performance(acpi_perf_data);
+	if (acpi_processor_preregister_performance(acpi_perf_data)) {
+		free_acpi_perf_data();
+		return -EINVAL;
+	}
+
 	return 0;
 }
 
would seem to be correct. The alternative is that the number of 
processors is also set to 0x80000000 - according to the spec "OSPM will 
not start performing power state transitions to a particular P-state 
until this number of processors belonging to the same domain have been 
detected and started". Since we're clearly never reaching that 
condition, why is the governor even getting started?

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [patch 1/3] acpi: check _PSS invalidation when BIOS report _PSS with 0x80000000

[Len Brown]

From: Len Brown <len.brown@intel.com>
Subject: [PATCH] ACPI: sanity check _PSS frequency to prevent cpufreq crash

When BIOS SETUP is changed to disable EIST, some BIOS
hand the OS an un-initialized _PSS:

        Name (_PSS, Package (0x06)
        {
            Package (0x06)
            {
                0x80000000,	// frequency [MHz]
                0x80000000,	// power [mW]
                0x80000000,	// latency [us]
                0x80000000,	// BM latency [us]
                0x80000000,	// control
                0x80000000	// status
            },
	    ...

These are outrageous values for frequency,
power and latency, raising the question where to draw
the line between legal and illegal.  We tend to survive
garbage in the power and latency fields, but we can BUG_ON
when garbage is in the frequency field.

Cpufreq multiplies the frequency by 1000 and stores it in a u32 KHz.
So disregard a _PSS with a frequency so large
that it can't be represented by cpufreq.

https://bugzilla.redhat.com/show_bug.cgi?id=500311

Signed-off-by: Len Brown <len.brown@intel.com>
---

I think this is a better patch for this issue.

thanks,
-Len

 drivers/acpi/processor_perflib.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
index cafb410..160f9ed 100644
--- a/drivers/acpi/processor_perflib.c
+++ b/drivers/acpi/processor_perflib.c
@@ -309,9 +309,13 @@ static int acpi_processor_get_performance_states(struct acpi_processor *pr)
 				  (u32) px->bus_master_latency,
 				  (u32) px->control, (u32) px->status));
 
-		if (!px->core_frequency) {
-			printk(KERN_ERR PREFIX
-				    "Invalid _PSS data: freq is zero\n");
+		/*
+ 		 * Check that ACPI's u64 MHz will be valid as u32 KHz in cpufreq
+		 */
+		if (!px->core_frequency ||
+			((u32)(px->core_frequency * 1000) != (px->core_frequency * 1000))) {
+			printk(KERN_ERR FW_BUG PREFIX
+				    "Invalid BIOS _PSS frequency: 0x%llx MHz\n", px->core_frequency);
 			result = -EFAULT;
 			kfree(pr->performance->states);
 			goto end;
-- 
1.6.3.1.169.g33fd