All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Renninger <trenn@suse.de>
To: "Brown, Len" <len.brown@intel.com>
Cc: Arjan van de Ven <arjan@linux.intel.com>,
	Pavel Machek <pavel@suse.cz>, Len Brown <lenb@kernel.org>,
	linux-acpi@vger.kernel.org, "Moore,
	Robert" <robert.moore@intel.com>,
	"Lin, Ming M" <ming.m.lin@intel.com>,
	Bjorn Helgaas <bjorn.helgaas@hp.com>,
	Huang Cheng <cheng.huang@intel.com>,
	firmwarekit-discuss@bughost.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: Check for BIOS bugs - Original Subject: Re: [PATCH 23/70] ACPICA: Workaround for reversed _PRT entries from BIOS
Date: Wed, 9 Jul 2008 10:51:55 +0200	[thread overview]
Message-ID: <200807091051.57322.trenn@suse.de> (raw)
In-Reply-To: <960B77DF2E6D974DBE24FD066EAEE8DAD90886@azsmsx422.amr.corp.intel.com>

On Tuesday 08 July 2008 21:34:55 Brown, Len wrote:
> >>>> Some BIOSs erroneously reverse the _PRT SourceName and the
> >>>> SourceIndex.  Detect and repair this problem. MS ACPI also allows
> >>>> and repairs this problem, thus ACPICA must also.
> >>>
> >>> It would be great to have an interface to report this as a
> >
> >BIOS defect.
> >
> >>> Something like:
> >>>
> >>> FIRMWARE_BUG_ON(FIRM_WARN, "erroneously reversed the _PRT
> >
> >source_name", ACPI_
> >
> >>> Bug);
> >>>
> >>> FIRMWARE_BUG_ON(severity, description, component);
> >>
> >> Yes, please.
>
> I'm not excited about maintaining
> maintaining linux-as-a-firmware-diagnostic --
> particularly when...
Above is not for ACPI only. But ACPI is probably a candidate which should make 
use of it.
>
> 1. it clutters the code for normail machines
It is the only disadvantage, "cluttering" the code.
On the other hand, this is an advantage.
While comments like "/* this must not happen, out of spec, etc. */"
should normally be added,
kernel developers tend to return -ENODEV and that's it.
A FIRMWARE_BUG("Description") would be compiled out on a
production kernel, compiled in for a kernel on which linuxfirmwarekit
boots or other debug kernels and would nicely document,
not clutter the code.

> 2. finding the bug is pointless,
No.
> because even 
>    if you fix one machine, you are guaranteed to
>    not fix all machines and thus must maintain
>    the workaround anyway.
I expect you want HP to fix their critical shut down temperature of 256 C?
If we would have known this, we had made them fix this and a lot other things 
also.
This is one of dozens of examples...

> >> I'd also like HARDWARE_BUG_ON(), with similar usage.
> >>
> >> With all the preload-linux-on-foo project, we have some
> >> chance to make
> >> BIOS vendors fix their stuff if we can easily diagnose errors in
> >> there.
>
> These customers should be running
> http://linuxfirmwarekit.org/
It is not worth much.
At least currently and a firmware bug interface to the kernel is about to 
change this.

1) linuxfirmwarekit is parsing dmesg..., these messages are changing
    all the time in the kernel.
    S390 AFAIK currently indexes all printks to avoid a simple space cleanup
    to mix up messges. It is not needed to go that far...
2) The rest is done through userspace plugins written in shell/perl or C.
     But most firmware bugs are absorbed by the kernel already. So there
     is nothing we could tell OEMs they should correct.
3) 2+3 is unmaintainable. There is enough work in the kernel, now we should
    write a linuxfirmwarekit plugin for every firmwarebug we find?!?
4) I tried to convince our kernel people to write linuxfirmwarekit plugins
     with exactly zero response. Hmm wrong, Frank wrote a thermal zone
    checker, some hundred lines of fragile shell code, it would have been less
    than ten lines in the kernel, just add some FIRMWARE_BUG(..) in the right
    if/else conditions. And it's not worth much anymore as soon as
    thermal_zone is moving to sysfs.

> We do maintain some degree of "high-road ACPI spec checking"
> with the "acpi=strict" boot option.  If we do more of this,
> I think it should stay under that option.
In the linuxfirmwarekit the ACPI tables are dumped, disassembled and 
recompiled. The errors and warnings are printed out as ACPI BIOS errors.
This works out.
The iasl compiler still is a bit too noisy and cannot disassemble SSDTs 
correctly (maybe it got fixed, don't know), but this can work out.

The problem is that you only find syntactical bugs.
But there are *a lot* bugs like e.g. critical temperature of 256 C. Or If this 
object exists, that object must exist. Logic ACPI BIOS bugs can only be found 
in the kernel itself at runtime.
Ahh Bjorn is in CC..., Wrong opregion declarations and PNP resource 
declarations are rather common. I mean theoretically you can find this in 
userspace. Not with disassembling and recompiling but with interpreting the 
tables in userspace... Again unmaintainable and who should do the checking?
The guy who writes the kernel code...

I really like the idea of the linuxfirmwarekit and tried to push it, but I do 
not see any future for it without kernel support.

       Thomas

  parent reply	other threads:[~2008-07-09  8:52 UTC|newest]

Thread overview: 100+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-20  7:43 ACPI patch queue for 2.6.27 Len Brown
2008-06-20  7:43 ` [PATCH 01/70] fix a deadlock issue when poking "eject" file Len Brown
2008-06-20  7:43   ` [PATCH 02/70] force offline the processor during hot-removal Len Brown
2008-06-20  7:43   ` [PATCH 03/70] create sysfs link from acpi device to sysdev for cpu Len Brown
2008-06-20  7:43   ` [PATCH 04/70] misc,acpi,backlight: compal Laptop Extras Len Brown
2008-06-20 10:13     ` Richard Purdie
2008-06-20 18:48       ` Len Brown
2008-06-20 11:48     ` Matthew Garrett
2008-06-20 18:52       ` Len Brown
2008-06-20 19:26         ` Matthew Garrett
2008-06-20 21:59           ` Henrique de Moraes Holschuh
2008-06-20 22:10             ` Matthew Garrett
2008-06-20 19:27       ` Henrique de Moraes Holschuh
2008-06-20  7:43   ` [PATCH 05/70] ACPI: change processors from array to per_cpu variable Len Brown
2008-06-20  7:43   ` [PATCH 06/70] Fujitsu-laptop update Len Brown
2008-06-20  7:43   ` [PATCH 07/70] ACPI PM: acpi_pm_device_sleep_state() cleanup Len Brown
2008-06-20  7:43   ` [PATCH 08/70] PCI ACPI: Drop the second argument of platform_pci_choose_state Len Brown
2008-06-20  7:43   ` [PATCH 09/70] ACPI PM: Remove obsolete Toshiba workaround Len Brown
2008-06-20  7:43   ` [PATCH 10/70] APM emulation: Notify about all suspend events, not just APM invoked ones (v2) Len Brown
2008-06-20  7:43   ` [PATCH 11/70] Freezer: Introduce PF_FREEZER_NOSIG Len Brown
2008-06-20  7:43   ` [PATCH 12/70] snapshot: Push BKL down into ioctl handlers Len Brown
2008-06-20  7:43   ` [PATCH 13/70] snapshot: Use pm_mutex for mutual exclusion Len Brown
2008-06-20  7:43   ` [PATCH 14/70] ACPICA: Add argument count checking to control method invocation via acpi_evaluate_object Len Brown
2008-06-20  7:43   ` [PATCH 15/70] ACPICA: Fix for hang on GPE method invocation Len Brown
2008-06-20  7:43   ` [PATCH 16/70] ACPICA: Update tracking macros to reduce code/data size Len Brown
2008-06-20  7:43   ` [PATCH 17/70] ACPICA: Fix possible negative array index in acpi_ut_validate_exception Len Brown
2008-06-20  7:43   ` [PATCH 18/70] ACPICA: Eliminate acpi_native_uint type Len Brown
2008-06-20  7:43   ` [PATCH 19/70] ACPICA: Removed unused include files from source files Len Brown
2008-06-20  7:43   ` [PATCH 20/70] ACPICA: Several lint changes, no functional changes Len Brown
2008-06-20  7:43   ` [PATCH 21/70] ACPICA: Add const qualifier for appropriate string constants Len Brown
2008-06-20  7:43   ` [PATCH 22/70] ACPICA: Update version to 20080514 Len Brown
2008-06-20  7:43   ` [PATCH 23/70] ACPICA: Workaround for reversed _PRT entries from BIOS Len Brown
2008-07-01  8:33     ` Check for BIOS bugs - Original Subject: " Thomas Renninger
2008-07-07 11:12       ` Pavel Machek
2008-07-08 15:40         ` Arjan van de Ven
2008-07-08 19:34           ` Brown, Len
2008-07-08 19:34             ` Brown, Len
2008-07-08 22:11             ` Pavel Machek
2008-07-09  8:51             ` Thomas Renninger [this message]
2008-07-17 20:31               ` Brown, Len
2008-07-17 20:31                 ` Brown, Len
2008-06-20  7:43   ` [PATCH 24/70] ACPICA: Update DMAR and SRAT table definitions Len Brown
2008-06-20  7:43   ` [PATCH 25/70] ACPICA: Update disassembler for DMAR table changes Len Brown
2008-06-20 11:52     ` Matthew Garrett
2008-06-20 18:41       ` Len Brown
2008-06-20 19:50         ` Andi Kleen
2008-06-26  4:41           ` Len Brown
2008-06-20 19:49     ` Andi Kleen
2008-06-21  5:56       ` Brown, Len
2008-06-20  7:43   ` [PATCH 26/70] ACPICA: Fix for invalid large array index on 64-bit systems Len Brown
2008-06-20  7:43   ` [PATCH 27/70] ACPICA: Cleanup debug operand dump mechanism Len Brown
2008-06-20  7:43   ` [PATCH 28/70] ACPICA: Cleanup of _PRT parsing code Len Brown
2008-06-20  7:43   ` [PATCH 29/70] ACPICA: Fix mutex debug code for wrong loop termination value Len Brown
2008-06-20  7:43   ` [PATCH 30/70] ACPICA: Update version to 20080609 Len Brown
2008-06-20  7:43   ` [PATCH 32/70] ACPI: fix checkpatch.pl complaints in scan.c Len Brown
2008-06-20 19:48     ` Andi Kleen
2008-06-20 20:32       ` Alok Kataria
2008-06-20 20:39         ` Andi Kleen
2008-06-20 20:59           ` Alok Kataria
2008-06-21 14:50             ` Andi Kleen
2008-06-20  7:43   ` [PATCH 33/70] ACPI: fix acpi fan state set error Len Brown
2008-06-20  7:43   ` [PATCH 34/70] ACPI Exception (video-1721): UNKNOWN_STATUS_CODE, Cant attach device Len Brown
2008-06-20  7:43   ` [PATCH 35/70] ACPI: no AC status notification Len Brown
2008-06-20  7:43   ` [PATCH 36/70] ACPICA: Fixes warning from exconfig.c on 64-bit build Len Brown
2008-06-20  7:43   ` [PATCH 37/70] ACPI: fix processor throttling set error Len Brown
2008-06-20  7:43   ` [PATCH 38/70] acpi: fix printk format warning Len Brown
2008-06-20  7:43   ` [PATCH 39/70] PNP: add detail to debug resource dump Len Brown
2008-06-20  7:43   ` [PATCH 40/70] PNP: remove pnp_resource.index Len Brown
2008-06-20  7:43   ` [PATCH 41/70] PNP: add pnp_resource_type() internal interface Len Brown
2008-06-20  7:43   ` [PATCH 42/70] PNP: add pnp_resource_type_name() helper function Len Brown
2008-06-20  7:43   ` [PATCH 43/70] PNP: make pnp_{port,mem,etc}_start(), et al work for invalid resources Len Brown
2008-06-20  7:43   ` [PATCH 44/70] PNP: replace pnp_resource_table with dynamically allocated resources Len Brown
2008-06-20  7:43   ` [PATCH 45/70] PNPACPI: keep disabled resources when parsing current config Len Brown
2008-06-20  7:43   ` [PATCH 46/70] PNP: remove ratelimit on add resource failures Len Brown
2008-06-20  7:43   ` [PATCH 47/70] PNP: dont sort by type in /sys/.../resources Len Brown
2008-06-20  7:43   ` [PATCH 48/70] PNP: add pnp_possible_config() -- can a device could be configured this way? Len Brown
2008-06-20  7:43   ` [PATCH 49/70] PNP: whitespace/coding style fixes Len Brown
2008-06-20  7:43   ` [PATCH 50/70] PNP: define PNP-specific IORESOURCE_IO_* flags alongside IRQ, DMA, MEM Len Brown
2008-06-20  7:43   ` [PATCH 51/70] PNP: make resource option structures private to PNP subsystem Len Brown
2008-06-20  7:43   ` [PATCH 52/70] PNP: introduce pnp_irq_mask_t typedef Len Brown
2008-06-20  7:43   ` [PATCH 53/70] PNP: increase I/O port & memory option address sizes Len Brown
2008-06-20  7:43   ` [PATCH 54/70] PNP: improve resource assignment debug Len Brown
2008-06-20  7:43   ` [PATCH 55/70] PNP: in debug resource dump, make empty list obvious Len Brown
2008-06-20  7:43   ` [PATCH 56/70] PNP: make resource assignment functions return 0 (success) or -EBUSY (failure) Len Brown
2008-06-20  7:43   ` [PATCH 57/70] PNP: remove redundant pnp_can_configure() check Len Brown
2008-06-20  7:44   ` [PATCH 58/70] PNP: centralize resource option allocations Len Brown
2008-06-20  7:44   ` [PATCH 59/70] PNPACPI: ignore _PRS interrupt numbers larger than PNP_IRQ_NR Len Brown
2008-06-20  7:44   ` [PATCH 60/70] PNP: rename pnp_register_*_resource() local variables Len Brown
2008-06-20  7:44   ` [PATCH 61/70] PNP: support optional IRQ resources Len Brown
2008-06-20  7:44   ` [PATCH 62/70] PNP: remove extra 0x100 bit from option priority Len Brown
2008-06-20  7:44   ` [PATCH 63/70] ISAPNP: handle independent options following dependent ones Len Brown
2008-06-20  7:44   ` [PATCH 64/70] PNP: convert resource options to single linked list Len Brown
2008-06-20  7:44   ` [PATCH 65/70] PNP: avoid legacy IDE IRQs Len Brown
2008-06-20  7:44   ` [PATCH 66/70] PNPACPI: add support for HP vendor-specific CCSR descriptors Len Brown
2008-06-20  7:44   ` [PATCH 67/70] ACPI: Disable the C2C3_FFH access mode HW has no MWAIT support Len Brown
2008-06-20  7:44   ` [PATCH 68/70] ACPI: Create "idle=halt" boot parameter Len Brown
2008-06-20  7:44   ` [PATCH 69/70] ACPI : Create "idle=nomwait" bootparam Len Brown
2008-06-20 15:02     ` Randy Dunlap
2008-06-20  7:44   ` [PATCH 70/70] ACPI : Add DMI check to disable mwait on broken Compal Len Brown
2008-06-23  0:24 ` ACPI patch queue for 2.6.27 Jonathan Woithe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200807091051.57322.trenn@suse.de \
    --to=trenn@suse.de \
    --cc=arjan@linux.intel.com \
    --cc=bjorn.helgaas@hp.com \
    --cc=cheng.huang@intel.com \
    --cc=firmwarekit-discuss@bughost.org \
    --cc=len.brown@intel.com \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ming.m.lin@intel.com \
    --cc=pavel@suse.cz \
    --cc=robert.moore@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.