[patch 2/6] acpi: check _PSS invalidation when BIOS report _PSS with 0x80000000

[patch 2/6] 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.

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
@@ -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:
_

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

[Len Brown]

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

On Fri, 9 Jan 2009, akpm@linux-foundation.org wrote:

> 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.
> 
> 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
> @@ -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:
> _
> 

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

[Andrew Morton]

On Fri, 09 Jan 2009 16:01:47 -0500 (EST)
Len Brown <lenb@kernel.org> 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!

RE: [patch 2/6] acpi: check _PSS invalidation when BIOS report _PSS with 0x80000000

[Pallipadi, Venkatesh]

>-----Original Message-----
>From: Andrew Morton [mailto:akpm@linux-foundation.org]
>Sent: Friday, January 09, 2009 1:24 PM
>To: Len Brown
>Cc: linux-acpi@vger.kernel.org; Song, Youquan; Pallipadi, Venkatesh
>Subject: Re: [patch 2/6] acpi: check _PSS invalidation when
>BIOS report _PSS with 0x80000000
>
>On Fri, 09 Jan 2009 16:01:47 -0500 (EST)
>Len Brown <lenb@kernel.org> 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

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

[Len Brown]

> On Fri, 09 Jan 2009 16:01:47 -0500 (EST)
> Len Brown <lenb@kernel.org> 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

RE: [patch 2/6] acpi: check _PSS invalidation when BIOS report _PSS with 0x80000000

[Len Brown]

> >> 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.

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