Skip to contentSkip to conversation

acpi: check _PSS invalidation when BIOS report _PSS with 0x80000000

PATCH Applied [email protected] 3 participants 6 messages Jan 9, 2009 ACPI

lore
Share & tools
lore

[email protected]

author
#0 lore raw

PATCH 2/6 acpi: check _PSS invalidation when BIOS report _PSS with 0x80000000

From: Youquan, Song <[email protected]>

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.

Signed-off-by Youquan, Song <[email protected]>
Signed-off-by Pallipadi, Venkatesh <[email protected]>
Cc Len Brown <[email protected]>
Signed-off-by Andrew Morton <[email protected]>

---

 drivers/acpi/processor_perflib.c |    9 +++++++++
 1 file changed, 9 insertions(+)
drivers/acpi/processor_perflib.c renamed from drivers/acpi/processor_perflib.c~acpi-check-_pss-invalidation-when-bios-report-_pss-with-all-0x80000000 +9 -0
diff -puN drivers/acpi/processor_perflib.c~acpi-check-_pss-invalidation-when-bios-report-_pss-with-all-0x80000000 drivers/acpi/processor_perflib.c
@@ -50,6 +50,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);

@@ -324,6 +325,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:

_

reviewer
#1 lore raw

I recall nacking this one some time ago.

ACPI_PROCESSOR_PSS_INVALID is fantasy.
Can't we be smarter about detecting and handling
BIOS bugs, and not pretend that the specification
tells us that this particular bit pattern is invalid (it does not) What happens when the next BIOS gives us 0x40000000?

thanks
Len Brown, Intel Open Source Technology Center

Quoted 46 lines · On Fri, 9 Jan 2009, [email protected] wrote:

From: Youquan, Song <[email protected]>

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.

Signed-off-by Youquan, Song <[email protected]>
Signed-off-by Pallipadi, Venkatesh <[email protected]>
Cc Len Brown <[email protected]>
Signed-off-by Andrew Morton <[email protected]>

---

 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

drivers/acpi/processor_perflib.c renamed from drivers/acpi/processor_perflib.c~acpi-check-_pss-invalidation-when-bios-report-_pss-with-all-0x80000000 +9 -0
@@ -50,6 +50,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);

@@ -324,6 +325,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:

_

author
#2 lore raw
Quoted 1 line · On Fri, 09 Jan 2009 16:01:47 -0500 (EST) Len Brown <[email protected]> wrote:

I recall nacking this one some time ago.

Well please treat it as a bug report then. "the kernel will boot crash when load cpufreq govenors". Bad!

#3 lore raw
Quoted 10 lines

-----Original Message-----
From: Andrew Morton [mailto:[email protected]]
Sent: Friday, January 09, 2009 1:24 PM
To: Len Brown
Cc: [email protected]; Song, Youquan; Pallipadi, Venkatesh
Subject: Re: [patch 2/6] acpi: check _PSS invalidation when BIOS report _PSS with 0x80000000

Quoted 8 lines
Quoted 1 line · On Fri, 09 Jan 2009 16:01:47 -0500 (EST) Len Brown <[email protected]> wrote:

I recall nacking this one some time ago.

Well please treat it as a bug report then. "the kernel will boot crash when load cpufreq govenors". Bad!

I think this should be a special case even when it is not defined in spec.

There is BIOS code out there that is giving 0x80000000 and Linux fails to boot on these systems, where as the other OS does not. Existence of even one such system means that the code will surely be copied onto other BIOSes and I don't think we can persuade BIOS guys to change this.

Thanks,
Venki

reviewer
#4 lore raw
Quoted 6 lines
Quoted 5 lines
Quoted 1 line

I recall nacking this one some time ago.

Well please treat it as a bug report then. "the kernel will boot crash when load cpufreq govenors". Bad!

Quoted 6 lines

I think this should be a special case even when it is not defined in spec.

There is BIOS code out there that is giving 0x80000000 and Linux fails to boot on these systems, where as the other OS does not. Existence of even one such system means that the code will surely be copied onto other BIOSes and I don't think we can persuade BIOS guys to change this.

Let me be clear.
I agree that this needs to be fixed.
My point is that this patch is the wrong way to do it.

We need to become hardened to total garbage in this field, or we need to do a better job of sanity checking it rather than checking one particular bit (of many) that may be bogus.

thanks,
-Len
Intel Open Source Technology Center

reviewer
#5 lore raw
In reply to Andrew Morton
Quoted 8 lines
Quoted 1 line · On Fri, 09 Jan 2009 16:01:47 -0500 (EST) Len Brown <[email protected]> wrote:

I recall nacking this one some time ago.

Well please treat it as a bug report then. "the kernel will boot crash when load cpufreq govenors". Bad!

I agree 100%
I recall asking what machine has this BIOS bug, and it was something like a prototype, so I didn't lose much sleep over it -- and it was a non-default configuration of that prototype...

I'm all for hardening Linux against BIOS bugs.
Checking 1 bad bit and not checking other bad
bits simply isn't the path to success.

thanks,
-Len
Intel Open Source Technology Center