| Message ID | 200901092017.n09KH9OP014905@imap1.linux-foundation.org (mailing list archive) |
|---|---|
| State | Rejected, archived |
| Headers | show |
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: > _ > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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!
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
>-----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 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> >> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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: