[PATCH] Hack libpci not to exit(1) during init if PCI does not work
This patch is for testing only, and not for immediate merge. libpci unhelpfully calls exit(1) instead of reporting an error if init fails. Fortunately, that part of libpci has not changed since the year 2000, so we can use a really evil hack to avoid that. We supply an alternative error printing function (which does not exit(1)) and this function detects which error is happening and hacks internal libpci structures in a way that avoids a later segfault due to NULL pointer dereference. We MUST absolutely make sure that pci_init() in libpci will not change or this hack will explode spectacularly. Once we decide that this hack is the way to go, notifying the libpci maintainer is essential. Apply this patch, then compile flashrom with make CONFIG_DUMMY=no CONFIG_INTERNAL=o CONFIG_SERPROG=no CONFIG_RAYER_SPI=no CONFIG_NIC3COM=no CONFIG_GFXNVIDIA=no CONFIG_SATASII=no CONFIG_FT2232_SPI=no CONFIG_DRKAISER=no CONFIG_NICREALTEK=no CONFIG_NICINTEL_SPI=no CONFIG_BUSPIRATE_SPI=no CONFIG_OGP_SPI=no CONFIG_SATAMV=no CONFIG_NICINTEL=yes To check if the hack works, run "flashrom -V" and respond to this mail with the output. You should get a message near the end which says that programmer initialization failed. Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> Index: flashrom-libpci_hack_avoid_exit1_during_init/pcidev.c =================================================================== --- flashrom-libpci_hack_avoid_exit1_during_init/pcidev.c (Revision 1362) +++ flashrom-libpci_hack_avoid_exit1_during_init/pcidev.c (Arbeitskopie) @@ -21,6 +21,8 @@ #include <stdlib.h> #include <string.h> +#include <stdio.h> +#include <stdarg.h> #include "flash.h" #include "programmer.h" @@ -188,6 +190,58 @@ return 0; } +/* Ugliest hack ever in flashrom! Should work for all libpci versions released + * in the year 2000 or later. Checked to make sense until at least version + * 3.1.8-test1. + */ +static void fake_libpci_method_init(struct pci_access *foo) +{ + msg_pinfo("Working around another pcilib crash\n"); + return; +} + +static struct { + char *name; /* name in all libpci versions */ + char *dummy1; /* Only present in some libpci versions */ + void *dummy2; + void (*init1)(struct pci_access *); /* init in older versions */ + void (*init2)(struct pci_access *); /* init in newer versions */ + void *dummy3; + void *dummy4; + void *dummy5; + void *dummy6; + void *dummy7; + void *dummy8; + void *dummy9; + void *dummy10; +} fake_libpci_methods = { + .name = "None", + .init1 = &fake_libpci_method_init, + .init2 = &fake_libpci_method_init, +}; + +static int pci_init_failed = 0; + +static void my_libpci_generic_error(char *msg, ...) +{ + va_list ap; + + msg_pinfo("pcilib: "); + va_start(ap, msg); + vprintf(msg, ap); + va_end(ap); + msg_pinfo("\n"); + if (!strcmp(msg, "Cannot find any working access method.")) { + /* Workaround for crash, very ugly. */ + msg_pinfo("Working around pcilib crash\n"); + pacc->methods = (void *)&fake_libpci_methods; + pci_init_failed = 1; + } else { + /* Any error in libpci would result in exit(1) by default. */ + exit(1); + } +} + uintptr_t pcidev_init(int bar, const struct pcidev_status *devs) { struct pci_dev *dev; @@ -198,7 +252,14 @@ uintptr_t addr = 0, curaddr = 0; pacc = pci_alloc(); /* Get the pci_access structure */ + pacc->error = &my_libpci_generic_error; pci_init(pacc); /* Initialize the PCI library */ + /* pci_init returns void, so we have to hack it. */ + if (pci_init_failed) { + /* Yes, technically we would have to free stuff... */ + return 1; + } + pci_scan_bus(pacc); /* We want to get the list of devices */ pci_filter_init(pacc, &filter); -- http://www.hailfinger.org/
On Sat, 02 Jul 2011 21:14:20 +0200 Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> wrote:
To check if the hack works, run "flashrom -V" and respond to this mail with the output. You should get a message near the end which says that programmer initialization failed.
sudo ./flashrom -V flashrom v0.9.3-r1362 on Linux 2.6.35-30-generic (x86_64), built with libpci 3.1.7, GCC 4.4.5, little endian flashrom is free software, get the source code at http://www.flashrom.org Calibrating delay loop... OS timer resolution is 1 usecs, 1445M loops per second, 10 myus = 10 us, 100 myus = 99 us, 1000 myus = 988 us, 10000 myus = 9931 us, 4 myus = 4 us, OK. Initializing nicintel programmer Error: No supported PCI device found. (on ubuntu 10.10) -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner
Am 03.07.2011 12:02 schrieb Stefan Tauner:
On Sat, 02 Jul 2011 21:14:20 +0200 Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> wrote:
To check if the hack works, run "flashrom -V" and respond to this mail with the output. You should get a message near the end which says that programmer initialization failed.
sudo ./flashrom -V flashrom v0.9.3-r1362 on Linux 2.6.35-30-generic (x86_64), built with libpci 3.1.7, GCC 4.4.5, little endian flashrom is free software, get the source code at http://www.flashrom.org
Calibrating delay loop... OS timer resolution is 1 usecs, 1445M loops per second, 10 myus = 10 us, 100 myus = 99 us, 1000 myus = 988 us, 10000 myus = 9931 us, 4 myus = 4 us, OK. Initializing nicintel programmer Error: No supported PCI device found.
Good, that means it still works as designed on machines with PCI. The error message I mentioned should only appear on non-PCI hosts. Regards, Carl-Daniel -- http://www.hailfinger.org/
On Sun, 03 Jul 2011 15:17:01 +0200 Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> wrote:
Am 03.07.2011 12:02 schrieb Stefan Tauner:
On Sat, 02 Jul 2011 21:14:20 +0200 Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> wrote:
To check if the hack works, run "flashrom -V" and respond to this mail with the output. You should get a message near the end which says that programmer initialization failed.
sudo ./flashrom -V flashrom v0.9.3-r1362 on Linux 2.6.35-30-generic (x86_64), built with libpci 3.1.7, GCC 4.4.5, little endian flashrom is free software, get the source code at http://www.flashrom.org
Calibrating delay loop... OS timer resolution is 1 usecs, 1445M loops per second, 10 myus = 10 us, 100 myus = 99 us, 1000 myus = 988 us, 10000 myus = 9931 us, 4 myus = 4 us, OK. Initializing nicintel programmer Error: No supported PCI device found.
Good, that means it still works as designed on machines with PCI. The error message I mentioned should only appear on non-PCI hosts.
i thought it was pretty pointless, but since you did not mention any prerequisites for testing i just gave it a try anyway :) -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner
New version: Should work for the internal programmer as well. TODO: Add sane error handling to pcidev_init() and its callers. libpci unhelpfully calls exit(1) instead of reporting an error if init fails. Fortunately, that part of libpci has not changed since the year 2000, so we can use a really evil hack to avoid that. We supply an alternative error printing function (which does not exit(1)) and this function detects which error is happening and hacks internal libpci structures in a way that avoids a later segfault due to NULL pointer dereference. We MUST absolutely make sure that pci_init() in libpci will not change or this hack will explode spectacularly. Once we decide that this hack is the way to go, notifying the libpci maintainer is essential. To check if the hack works, run "flashrom -V" and respond to this mail with the output. If you don't have working PCI access, you should get a message near the end which says that programmer initialization failed. If you have working PCI access, the patch should not change anything. Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> Index: flashrom-libpci_hack_avoid_exit1_during_init/pcidev.c =================================================================== --- flashrom-libpci_hack_avoid_exit1_during_init/pcidev.c (Revision 1362) +++ flashrom-libpci_hack_avoid_exit1_during_init/pcidev.c (Arbeitskopie) @@ -21,6 +21,8 @@ #include <stdlib.h> #include <string.h> +#include <stdio.h> +#include <stdarg.h> #include "flash.h" #include "programmer.h" @@ -188,6 +190,77 @@ return 0; } +/* Ugliest hack ever in flashrom! Should work for all libpci versions released + * in the year 2000 or later. Checked to make sense until at least version + * 3.1.8-test1. + */ +static void fake_libpci_method_init(struct pci_access *foo) +{ + msg_pinfo("Working around another pcilib crash\n"); + return; +} + +static struct { + char *name; /* name in all libpci versions */ + char *dummy1; /* Only present in some libpci versions */ + void *dummy2; + void (*init1)(struct pci_access *); /* init in older versions */ + void (*init2)(struct pci_access *); /* init in newer versions */ + void *dummy3; + void *dummy4; + void *dummy5; + void *dummy6; + void *dummy7; + void *dummy8; + void *dummy9; + void *dummy10; +} fake_libpci_methods = { + .name = "None", + .init1 = &fake_libpci_method_init, + .init2 = &fake_libpci_method_init, +}; + +static int pci_init_failed = 0; + +static void my_libpci_generic_error(char *msg, ...) +{ + va_list ap; + + msg_pinfo("pcilib: "); + va_start(ap, msg); + vprintf(msg, ap); + va_end(ap); + msg_pinfo("\n"); + if (!strcmp(msg, "Cannot find any working access method.")) { + /* Workaround for crash, very ugly. */ + msg_pinfo("Working around pcilib crash\n"); + pacc->methods = (void *)&fake_libpci_methods; + pci_init_failed = 1; + } else { + /* Any error in libpci would result in exit(1) by default. */ + exit(1); + } +} + +/* Wrapper for libpci pci_init() which simply calls exit(1) on error. */ +int flashrom_pci_init(void) +{ + /* Allocate the pci_access structure. */ + pacc = pci_alloc(); + /* Hook the error printing function which would exit(1). */ + pacc->error = &my_libpci_generic_error; + /* Initialize the PCI library. */ + pci_init(pacc); + /* pci_init returns void, so we have to get any error status from the + * hooked pacc->error which updates pci_init_failed. + */ + if (pci_init_failed) { + pci_cleanup(pacc); + return 1; + } + return 0; +} + uintptr_t pcidev_init(int bar, const struct pcidev_status *devs) { struct pci_dev *dev; @@ -197,8 +270,9 @@ int found = 0; uintptr_t addr = 0, curaddr = 0; - pacc = pci_alloc(); /* Get the pci_access structure */ - pci_init(pacc); /* Initialize the PCI library */ + if (flashrom_pci_init()) + return 1; + pci_scan_bus(pacc); /* We want to get the list of devices */ pci_filter_init(pacc, &filter); Index: flashrom-libpci_hack_avoid_exit1_during_init/internal.c =================================================================== --- flashrom-libpci_hack_avoid_exit1_during_init/internal.c (Revision 1362) +++ flashrom-libpci_hack_avoid_exit1_during_init/internal.c (Arbeitskopie) @@ -192,10 +192,8 @@ */ buses_supported = CHIP_BUSTYPE_NONSPI; - /* Initialize PCI access for flash enables */ - pacc = pci_alloc(); /* Get the pci_access structure */ - /* Set all options you want -- here we stick with the defaults */ - pci_init(pacc); /* Initialize the PCI library */ + if (flashrom_pci_init()) + return 1; pci_scan_bus(pacc); /* We want to get the list of devices */ if (processor_flash_enable()) { Index: flashrom-libpci_hack_avoid_exit1_during_init/programmer.h =================================================================== --- flashrom-libpci_hack_avoid_exit1_during_init/programmer.h (Revision 1362) +++ flashrom-libpci_hack_avoid_exit1_during_init/programmer.h (Arbeitskopie) @@ -225,6 +225,7 @@ }; uintptr_t pcidev_validate(struct pci_dev *dev, int bar, const struct pcidev_status *devs); uintptr_t pcidev_init(int bar, const struct pcidev_status *devs); +int flashrom_pci_init(void); /* rpci_write_* are reversible writes. The original PCI config space register * contents will be restored on shutdown. */ -- http://www.hailfinger.org/
Impressive! On Wed, Jul 6, 2011 at 3:34 PM, Carl-Daniel Hailfinger < c-d.hailfinger.devel.2006@gmx.net> wrote:
- /* Initialize PCI access for flash enables */
- pacc = pci_alloc(); /* Get the pci_access structure */
- /* Set all options you want -- here we stick with the defaults */ - pci_init(pacc); /* Initialize the PCI library */ + if (flashrom_pci_init()) + return 1; pci_scan_bus(pacc); /* We want to get the list of devices */
Change those last three lines to: if (!flashrom_pci_init()) pci_scan_bus(pacc); and it works on systems w/o PCI (at least it did on ARM) Here are my test results with this patch and the tegra2 patch: http://paste.flashrom.org/view.php?id=683 -- David Hendricks (dhendrix) Systems Software Engineer, Google Inc.
On Thu, Jul 07, 2011 at 12:34:55AM +0200, Carl-Daniel Hailfinger wrote:
New version: Should work for the internal programmer as well.
TODO: Add sane error handling to pcidev_init() and its callers.
Yep, but that's for another patch.
Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
Acked-by: Uwe Hermann <uwe@hermann-uwe.de> I see no reason not to include this.
+static struct { + char *name; /* name in all libpci versions */ + char *dummy1; /* Only present in some libpci versions */ + void *dummy2; + void (*init1)(struct pci_access *); /* init in older versions */ + void (*init2)(struct pci_access *); /* init in newer versions */ + void *dummy3; + void *dummy4; + void *dummy5; + void *dummy6; + void *dummy7; + void *dummy8; + void *dummy9; + void *dummy10;
Indentation should be one tab above too. Attached are two logs, one from a normal board, one from a non-supported laptop with force. I can make some more different logs if you like. Uwe. -- http://hermann-uwe.de | http://sigrok.org http://randomprojects.org | http://unmaintained-free-software.org
On Thu, 07 Jul 2011 00:34:55 +0200 Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> wrote:
New version: Should work for the internal programmer as well.
TODO: Add sane error handling to pcidev_init() and its callers.
libpci unhelpfully calls exit(1) instead of reporting an error if init fails. Fortunately, that part of libpci has not changed since the year 2000, so we can use a really evil hack to avoid that. We supply an alternative error printing function (which does not exit(1)) and this function detects which error is happening and hacks internal libpci structures in a way that avoids a later segfault due to NULL pointer dereference.
We MUST absolutely make sure that pci_init() in libpci will not change or this hack will explode spectacularly. Once we decide that this hack is the way to go, notifying the libpci maintainer is essential.
forgive me my naivety, but was upstream ever asked if they could mitigate the problem (at least in the long term)? just exiting is of course pretty much insane for any library... i would presume that they understand that. the git shows even less commits than in flashrom, so i think we would have to do it on our own... but still better than the hack... in the long term :) -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner
participants (4)
-
Carl-Daniel Hailfinger -
David Hendricks -
Stefan Tauner -
Uwe Hermann