Hello flashrom developers,
please find included the DMI matching patch.
The patch has been tested to work on my Fujitsu S7110 laptop with fake board enable entries. Non-availability of dmidecode has also been tested to work as expected.
Regards, Michael Karcher
Matching via DMI: If a board is not uniquely identifiable by PCI device/subsystem IDs, a string can be specified to be looked for (case-sensitive, substring) for now in one of the following items: - System Manufacturer - System Product Name - System Version - Baseboard Manufacturer - Baseboard Product Name - Baseboard Version
The match is only made if DMI info is available and the string matches. If no DMI info is available, a warning is printed as the board can not be autodetected.
It's still open to discussion whether we add an DMI override switch to specify a string that will definitely match, and whether this switch is only used if no DMI is available or whether it overrides or augments DMI data.
DMI data is currently read using dmidecode. This tool is available for all major platforms except MacOS X. I heard that there also is a MacOS X version of dmidecode, but didn't investigate that.
Signed-Off-By: Michael Karcher flashrom@mkarcher.dialup.fu-berlin.de
Index: flash.h =================================================================== --- flash.h (Revision 824) +++ flash.h (Arbeitskopie) @@ -251,6 +251,9 @@ uint16_t second_card_vendor; uint16_t second_card_device;
+ /* String to appear in the DMI info */ + const char *dmi_identifier; + /* The vendor / part name from the coreboot table. */ const char *lb_vendor; const char *lb_part; @@ -330,6 +333,11 @@ extern char *lb_part, *lb_vendor; extern int partvendor_from_cbtable;
+/* dmi.c */ +extern int has_dmi_support; +void dmi_init(void); +int dmi_match(const char * identifier); + /* internal.c */ #if NEED_PCI == 1 struct superio { Index: Makefile =================================================================== --- Makefile (Revision 824) +++ Makefile (Arbeitskopie) @@ -101,7 +101,7 @@
ifeq ($(CONFIG_INTERNAL), yes) FEATURE_CFLAGS += -D'INTERNAL_SUPPORT=1' -PROGRAMMER_OBJS += chipset_enable.o board_enable.o cbtable.o it87spi.o ichspi.o sb600spi.o wbsio_spi.o +PROGRAMMER_OBJS += chipset_enable.o board_enable.o cbtable.o dmi.o it87spi.o ichspi.o sb600spi.o wbsio_spi.o NEED_PCI := yes endif
Index: dmi.c =================================================================== --- dmi.c (Revision 0) +++ dmi.c (Revision 0) @@ -0,0 +1,82 @@ +/* + * This file is part of the flashrom project. + * + * Copyright (C) 2009 Michael Karcher + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include <string.h> +#include <stdio.h> + +#include "flash.h" + +enum dmi_strings {DMI_SYS_VENDOR, DMI_SYS_PRODUCT, DMI_SYS_VERSION, + DMI_BB_VENDOR, DMI_BB_PRODUCT, DMI_BB_VERSION, + DMI_MAX_ID = DMI_BB_VERSION}; + +const char * const dmidecode_names[DMI_MAX_ID+1] = + {"system-manufacturer", "system-product-name", + "system-version", "baseboard-manufacturer", + "baseboard-product-name", "baseboard-version"}; + +int has_dmi_support = 0; +char dmistrings[DMI_MAX_ID+1][80]; + +void dmi_init(void) +{ + FILE * dmidecode_pipe; + int i; + for(i = 0; i <= DMI_MAX_ID; i++) + { + char commandline[40]; + sprintf(commandline, "dmidecode -s %s", dmidecode_names[i]); + dmidecode_pipe = popen(commandline, "r"); + fgets(dmistrings[i], sizeof(dmistrings[i]), dmidecode_pipe); + if(ferror(dmidecode_pipe)) + { + printf_debug("DMI pipe read error"); + pclose(dmidecode_pipe); + return; + } + /* Toss all output above 80 characters away to prevent + deadlock on pclose. */ + while(!feof(dmidecode_pipe)) + getc(dmidecode_pipe); + if(pclose(dmidecode_pipe) != 0) + { + printf_debug("DMI pipe close error"); + return; + } + + /* chomp trailing newline */ + if(dmistrings[i][0] != 0 && + dmistrings[i][strlen(dmistrings[i]) - 1] == '\n') + dmistrings[i][strlen(dmistrings[i]) - 1] = 0; + printf_debug("DMI string %d: %s\n", i, dmistrings[i]); + } + has_dmi_support = 1; +} + +int dmi_match(const char * identifier) +{ + int i; + if(!has_dmi_support) + return 0; + for(i = 0;i <= DMI_MAX_ID; i++) + if(strstr(dmistrings[i], identifier)) + return 1; + return 0; +} Index: internal.c =================================================================== --- internal.c (Revision 824) +++ internal.c (Arbeitskopie) @@ -152,6 +152,7 @@ * mainboard specific flash enable sequence. */ coreboot_init(); + dmi_init();
/* Probe for the SuperI/O chip and fill global struct superio. */ probe_superio(); Index: board_enable.c =================================================================== --- board_enable.c (Revision 824) +++ board_enable.c (Arbeitskopie) @@ -1162,6 +1162,11 @@ * provide an as complete set of pci ids as possible; autodetection is the * preferred behaviour and we would like to make sure that matches are unique. * + * If PCI IDs are not sufficient for board matching, the match can be further + * constrained by a string that has to be present in the DMI database for + * the baseboard or the system entry. The match type is case-sensitive + * substring match. + * * The coreboot ids are used two fold. When running with a coreboot firmware, * the ids uniquely matches the coreboot board identification string. When a * legacy bios is installed and when autodetection is not possible, these ids @@ -1173,59 +1178,59 @@
/* Please keep this list alphabetically ordered by vendor/board name. */ struct board_pciid_enable board_pciid_enables[] = { - /* first pci-id set [4], second pci-id set [4], coreboot id [2], vendor name board name flash enable */ - {0x8086, 0x2926, 0x147b, 0x1084, 0x11ab, 0x4364, 0x147b, 0x1084, NULL, NULL, "Abit", "IP35", intel_ich_gpio16_raise}, - {0x105a, 0x0d30, 0x105a, 0x4d33, 0x8086, 0x1130, 0x8086, 0, NULL, NULL, "Acorp", "6A815EPD", board_acorp_6a815epd}, - {0x8086, 0x24D4, 0x1849, 0x24D0, 0x8086, 0x24D5, 0x1849, 0x9739, NULL, NULL, "ASRock", "P4i65GV", intel_ich_gpio23_raise}, - {0x1022, 0x746B, 0, 0, 0, 0, 0, 0, "AGAMI", "ARUMA", "agami", "Aruma", w83627hf_gpio24_raise_2e}, - {0x1106, 0x3177, 0x17F2, 0x3177, 0x1106, 0x3148, 0x17F2, 0x3148, NULL, NULL, "Albatron", "PM266A", w836xx_memw_enable_2e}, - {0x1106, 0x3205, 0x1106, 0x3205, 0x10EC, 0x8139, 0xA0A0, 0x0477, NULL, NULL, "AOpen", "vKM400Am-S", it8705_rom_write_enable}, - {0x1022, 0x2090, 0, 0, 0x1022, 0x2080, 0, 0, "artecgroup", "dbe61", "Artec Group", "DBE61", board_artecgroup_dbe6x}, - {0x1022, 0x2090, 0, 0, 0x1022, 0x2080, 0, 0, "artecgroup", "dbe62", "Artec Group", "DBE62", board_artecgroup_dbe6x}, - {0x1106, 0x3189, 0x1043, 0x807F, 0x1106, 0x3065, 0x1043, 0x80ED, NULL, NULL, "ASUS", "A7V600-X", board_asus_a7v600x}, - {0x1106, 0x3189, 0x1043, 0x807F, 0x1106, 0x3177, 0x1043, 0x808C, NULL, NULL, "ASUS", "A7V8X", board_asus_a7v8x}, - {0x1106, 0x3177, 0x1043, 0x80A1, 0x1106, 0x3205, 0x1043, 0x8118, NULL, NULL, "ASUS", "A7V8X-MX SE", w836xx_memw_enable_2e}, - {0x1106, 0x1336, 0x1043, 0x80ed, 0x1106, 0x3288, 0x1043, 0x8249, NULL, NULL, "ASUS", "M2V-MX", via_vt823x_gpio5_raise}, - {0x8086, 0x1a30, 0x1043, 0x8070, 0x8086, 0x244b, 0x1043, 0x8028, NULL, NULL, "ASUS", "P4B266", intel_ich_gpio22_raise}, - {0x8086, 0x1A30, 0x1043, 0x8025, 0x8086, 0x244B, 0x104D, 0x80F0, NULL, NULL, "ASUS", "P4B266-LM", intel_ich_gpio21_raise}, - {0x8086, 0x2570, 0x1043, 0x80F2, 0x105A, 0x3373, 0x1043, 0x80F5, NULL, NULL, "ASUS", "P4P800-E Deluxe", intel_ich_gpio21_raise}, - {0x10B9, 0x1541, 0, 0, 0x10B9, 0x1533, 0, 0, "asus", "p5a", "ASUS", "P5A", board_asus_p5a}, - {0x10DE, 0x0030, 0x1043, 0x818a, 0x8086, 0x100E, 0x1043, 0x80EE, NULL, NULL, "ASUS", "P5ND2-SLI Deluxe", nvidia_mcp_gpio10_raise}, - {0x1106, 0x3149, 0x1565, 0x3206, 0x1106, 0x3344, 0x1565, 0x1202, NULL, NULL, "Biostar", "P4M80-M4", it8705_rom_write_enable}, - {0x8086, 0x3590, 0x1028, 0x016c, 0x1000, 0x0030, 0x1028, 0x016c, NULL, NULL, "Dell", "PowerEdge 1850", intel_ich_gpio23_raise}, - {0x1039, 0x5513, 0x1019, 0x0A41, 0x1039, 0x0018, 0, 0, NULL, NULL, "Elitegroup", "K7S6A", elitegroup_k7vta3}, - {0x1106, 0x3038, 0x1019, 0x0996, 0x1106, 0x3177, 0x1019, 0x0996, NULL, NULL, "Elitegroup", "K7VTA3", elitegroup_k7vta3}, - {0x1106, 0x3177, 0x1106, 0x3177, 0x1106, 0x3059, 0x1695, 0x3005, NULL, NULL, "EPoX", "EP-8K5A2", w836xx_memw_enable_2e}, - {0x10EC, 0x8139, 0x1695, 0x9001, 0x11C1, 0x5811, 0x1695, 0x9015, NULL, NULL, "EPoX", "EP-8RDA3+", nvidia_mcp_gpio31_raise}, - {0x8086, 0x7110, 0, 0, 0x8086, 0x7190, 0, 0, "epox", "ep-bx3", "EPoX", "EP-BX3", board_epox_ep_bx3}, - {0x1039, 0x0761, 0, 0, 0x10EC, 0x8168, 0, 0, "gigabyte", "2761gxdk", "GIGABYTE", "GA-2761GXDK", it87xx_probe_spi_flash}, - {0x1106, 0x3227, 0x1458, 0x5001, 0x10ec, 0x8139, 0x1458, 0xe000, NULL, NULL, "GIGABYTE", "GA-7VT600", it8705_rom_write_enable}, - {0x10DE, 0x0050, 0x1458, 0x0C11, 0x10DE, 0x005e, 0x1458, 0x5000, NULL, NULL, "GIGABYTE", "GA-K8N-SLI", nvidia_mcp_gpio21_raise}, - {0x10DE, 0x0360, 0x1458, 0x0C11, 0x10DE, 0x0369, 0x1458, 0x5001, "gigabyte", "m57sli", "GIGABYTE", "GA-M57SLI-S4", it87xx_probe_spi_flash}, - {0x10de, 0x03e0, 0, 0, 0x10DE, 0x03D0, 0, 0, NULL, NULL, "GIGABYTE", "GA-M61P-S3", it87xx_probe_spi_flash}, - {0x1002, 0x4398, 0x1458, 0x5004, 0x1002, 0x4391, 0x1458, 0xb000, NULL, NULL, "GIGABYTE", "GA-MA78G-DS3H", it87xx_probe_spi_flash}, - {0x1002, 0x4398, 0x1458, 0x5004, 0x1002, 0x4391, 0x1458, 0xb002, NULL, NULL, "GIGABYTE", "GA-MA78GM-S2H", it87xx_probe_spi_flash}, - {0x1002, 0x438d, 0x1458, 0x5001, 0x1002, 0x5956, 0x1002, 0x5956, NULL, NULL, "GIGABYTE", "GA-MA790FX-DQ6", it87xx_probe_spi_flash}, - {0x1166, 0x0223, 0x103c, 0x320d, 0x102b, 0x0522, 0x103c, 0x31fa, "hp", "dl145_g3", "HP", "DL145 G3", board_hp_dl145_g3_enable}, - {0x1166, 0x0205, 0x1014, 0x0347, 0x1002, 0x515E, 0x1014, 0x0325, NULL, NULL, "IBM", "x3455", board_ibm_x3455}, - {0x1039, 0x5513, 0x8086, 0xd61f, 0x1039, 0x6330, 0x8086, 0xd61f, NULL, NULL, "Intel", "D201GLY", wbsio_check_for_spi}, - {0x1022, 0x7468, 0, 0, 0, 0, 0, 0, "iwill", "dk8_htx", "IWILL", "DK8-HTX", w83627hf_gpio24_raise_2e}, - {0x8086, 0x27A0, 0, 0, 0x8086, 0x27b8, 0, 0, "kontron", "986lcd-m", "Kontron", "986LCD-M", board_kontron_986lcd_m}, - {0x8086, 0x2411, 0x8086, 0x2411, 0x8086, 0x7125, 0x0e11, 0xb165, NULL, NULL, "Mitac", "6513WU", board_mitac_6513wu}, - {0x13f6, 0x0111, 0x1462, 0x5900, 0x1106, 0x3177, 0x1106, 0, NULL, NULL, "MSI", "MS-6590 (KT4 Ultra)",board_msi_kt4v}, - {0x1106, 0x3149, 0x1462, 0x7094, 0x10ec, 0x8167, 0x1462, 0x094c, NULL, NULL, "MSI", "MS-6702E (K8T Neo2-F)",w83627thf_gpio4_4_raise_2e}, - {0x1106, 0x0571, 0x1462, 0x7120, 0x1106, 0x3065, 0x1462, 0x7120, NULL, NULL, "MSI", "MS-6712 (KT4V)", board_msi_kt4v}, - {0x8086, 0x2658, 0x1462, 0x7046, 0x1106, 0x3044, 0x1462, 0x046d, NULL, NULL, "MSI", "MS-7046", intel_ich_gpio19_raise}, - {0x10DE, 0x005E, 0x1462, 0x7135, 0x10DE, 0x0050, 0x1462, 0x7135, "msi", "k8n-neo3", "MSI", "MS-7135 (K8N Neo3)", w83627thf_gpio4_4_raise_4e}, - {0x10DE, 0x005E, 0x1462, 0x7125, 0x10DE, 0x0052, 0x1462, 0x7125, NULL, NULL, "MSI", "K8N Neo4-F", nvidia_mcp_gpio2_raise}, - {0x1106, 0x3099, 0, 0, 0x1106, 0x3074, 0, 0, "shuttle", "ak31", "Shuttle", "AK31", w836xx_memw_enable_2e}, - {0x1106, 0x3104, 0x1297, 0xa238, 0x1106, 0x3059, 0x1297, 0xc063, NULL, NULL, "Shuttle", "AK38N", shuttle_ak38n}, - {0x10DE, 0x0050, 0x1297, 0x5036, 0x1412, 0x1724, 0x1297, 0x5036, NULL, NULL, "Shuttle", "FN25", board_shuttle_fn25}, - {0x1106, 0x3038, 0x0925, 0x1234, 0x1106, 0x3058, 0x15DD, 0x7609, NULL, NULL, "Soyo", "SY-7VCA", board_soyo_sy_7vca}, - {0x8086, 0x1076, 0x8086, 0x1176, 0x1106, 0x3059, 0x10f1, 0x2498, NULL, NULL, "Tyan", "S2498 (Tomcat K7M)", w836xx_memw_enable_2e}, - {0x1106, 0x3177, 0x1106, 0xAA01, 0x1106, 0x3123, 0x1106, 0xAA01, NULL, NULL, "VIA", "EPIA M/MII/...", via_vt823x_gpio15_raise}, - {0x1106, 0x0259, 0x1106, 0x3227, 0x1106, 0x3065, 0x1106, 0x3149, NULL, NULL, "VIA", "EPIA-N/NL", via_vt823x_gpio9_raise}, - {0x1106, 0x5337, 0x1458, 0xb003, 0x1106, 0x287e, 0x1106, 0x337e, NULL, NULL, "VIA", "PC3500G", it87xx_probe_spi_flash}, + /* first pci-id set [4], second pci-id set [4], dmi_identifier, coreboot id [2], vendor name board name flash enable */ + {0x8086, 0x2926, 0x147b, 0x1084, 0x11ab, 0x4364, 0x147b, 0x1084, NULL, NULL, NULL, "Abit", "IP35", intel_ich_gpio16_raise}, + {0x105a, 0x0d30, 0x105a, 0x4d33, 0x8086, 0x1130, 0x8086, 0, NULL, NULL, NULL, "Acorp", "6A815EPD", board_acorp_6a815epd}, + {0x8086, 0x24D4, 0x1849, 0x24D0, 0x8086, 0x24D5, 0x1849, 0x9739, NULL, NULL, NULL, "ASRock", "P4i65GV", intel_ich_gpio23_raise}, + {0x1022, 0x746B, 0, 0, 0, 0, 0, 0, NULL, "AGAMI", "ARUMA", "agami", "Aruma", w83627hf_gpio24_raise_2e}, + {0x1106, 0x3177, 0x17F2, 0x3177, 0x1106, 0x3148, 0x17F2, 0x3148, NULL, NULL, NULL, "Albatron", "PM266A", w836xx_memw_enable_2e}, + {0x1106, 0x3205, 0x1106, 0x3205, 0x10EC, 0x8139, 0xA0A0, 0x0477, NULL, NULL, NULL, "AOpen", "vKM400Am-S", it8705_rom_write_enable}, + {0x1022, 0x2090, 0, 0, 0x1022, 0x2080, 0, 0, NULL, "artecgroup", "dbe61", "Artec Group", "DBE61", board_artecgroup_dbe6x}, + {0x1022, 0x2090, 0, 0, 0x1022, 0x2080, 0, 0, NULL, "artecgroup", "dbe62", "Artec Group", "DBE62", board_artecgroup_dbe6x}, + {0x1106, 0x3189, 0x1043, 0x807F, 0x1106, 0x3065, 0x1043, 0x80ED, NULL, NULL, NULL, "ASUS", "A7V600-X", board_asus_a7v600x}, + {0x1106, 0x3189, 0x1043, 0x807F, 0x1106, 0x3177, 0x1043, 0x808C, NULL, NULL, NULL, "ASUS", "A7V8X", board_asus_a7v8x}, + {0x1106, 0x3177, 0x1043, 0x80A1, 0x1106, 0x3205, 0x1043, 0x8118, NULL, NULL, NULL, "ASUS", "A7V8X-MX SE", w836xx_memw_enable_2e}, + {0x1106, 0x1336, 0x1043, 0x80ed, 0x1106, 0x3288, 0x1043, 0x8249, NULL, NULL, NULL, "ASUS", "M2V-MX", via_vt823x_gpio5_raise}, + {0x8086, 0x1a30, 0x1043, 0x8070, 0x8086, 0x244b, 0x1043, 0x8028, NULL, NULL, NULL, "ASUS", "P4B266", intel_ich_gpio22_raise}, + {0x8086, 0x1A30, 0x1043, 0x8025, 0x8086, 0x244B, 0x104D, 0x80F0, NULL, NULL, NULL, "ASUS", "P4B266-LM", intel_ich_gpio21_raise}, + {0x8086, 0x2570, 0x1043, 0x80F2, 0x105A, 0x3373, 0x1043, 0x80F5, NULL, NULL, NULL, "ASUS", "P4P800-E Deluxe", intel_ich_gpio21_raise}, + {0x10B9, 0x1541, 0, 0, 0x10B9, 0x1533, 0, 0, NULL, "asus", "p5a", "ASUS", "P5A", board_asus_p5a}, + {0x10DE, 0x0030, 0x1043, 0x818a, 0x8086, 0x100E, 0x1043, 0x80EE, NULL, NULL, NULL, "ASUS", "P5ND2-SLI Deluxe", nvidia_mcp_gpio10_raise}, + {0x1106, 0x3149, 0x1565, 0x3206, 0x1106, 0x3344, 0x1565, 0x1202, NULL, NULL, NULL, "Biostar", "P4M80-M4", it8705_rom_write_enable}, + {0x8086, 0x3590, 0x1028, 0x016c, 0x1000, 0x0030, 0x1028, 0x016c, NULL, NULL, NULL, "Dell", "PowerEdge 1850", intel_ich_gpio23_raise}, + {0x1039, 0x5513, 0x1019, 0x0A41, 0x1039, 0x0018, 0, 0, NULL, NULL, NULL, "Elitegroup", "K7S6A", elitegroup_k7vta3}, + {0x1106, 0x3038, 0x1019, 0x0996, 0x1106, 0x3177, 0x1019, 0x0996, NULL, NULL, NULL, "Elitegroup", "K7VTA3", elitegroup_k7vta3}, + {0x1106, 0x3177, 0x1106, 0x3177, 0x1106, 0x3059, 0x1695, 0x3005, NULL, NULL, NULL, "EPoX", "EP-8K5A2", w836xx_memw_enable_2e}, + {0x10EC, 0x8139, 0x1695, 0x9001, 0x11C1, 0x5811, 0x1695, 0x9015, NULL, NULL, NULL, "EPoX", "EP-8RDA3+", nvidia_mcp_gpio31_raise}, + {0x8086, 0x7110, 0, 0, 0x8086, 0x7190, 0, 0, NULL, "epox", "ep-bx3", "EPoX", "EP-BX3", board_epox_ep_bx3}, + {0x1039, 0x0761, 0, 0, 0x10EC, 0x8168, 0, 0, NULL, "gigabyte", "2761gxdk", "GIGABYTE", "GA-2761GXDK", it87xx_probe_spi_flash}, + {0x1106, 0x3227, 0x1458, 0x5001, 0x10ec, 0x8139, 0x1458, 0xe000, NULL, NULL, NULL, "GIGABYTE", "GA-7VT600", it8705_rom_write_enable}, + {0x10DE, 0x0050, 0x1458, 0x0C11, 0x10DE, 0x005e, 0x1458, 0x5000, NULL, NULL, NULL, "GIGABYTE", "GA-K8N-SLI", nvidia_mcp_gpio21_raise}, + {0x10DE, 0x0360, 0x1458, 0x0C11, 0x10DE, 0x0369, 0x1458, 0x5001, NULL, "gigabyte", "m57sli", "GIGABYTE", "GA-M57SLI-S4", it87xx_probe_spi_flash}, + {0x10de, 0x03e0, 0, 0, 0x10DE, 0x03D0, 0, 0, NULL, NULL, NULL, "GIGABYTE", "GA-M61P-S3", it87xx_probe_spi_flash}, + {0x1002, 0x4398, 0x1458, 0x5004, 0x1002, 0x4391, 0x1458, 0xb000, NULL, NULL, NULL, "GIGABYTE", "GA-MA78G-DS3H", it87xx_probe_spi_flash}, + {0x1002, 0x4398, 0x1458, 0x5004, 0x1002, 0x4391, 0x1458, 0xb002, NULL, NULL, NULL, "GIGABYTE", "GA-MA78GM-S2H", it87xx_probe_spi_flash}, + {0x1002, 0x438d, 0x1458, 0x5001, 0x1002, 0x5956, 0x1002, 0x5956, NULL, NULL, NULL, "GIGABYTE", "GA-MA790FX-DQ6", it87xx_probe_spi_flash}, + {0x1166, 0x0223, 0x103c, 0x320d, 0x102b, 0x0522, 0x103c, 0x31fa, NULL, "hp", "dl145_g3", "HP", "DL145 G3", board_hp_dl145_g3_enable}, + {0x1166, 0x0205, 0x1014, 0x0347, 0x1002, 0x515E, 0x1014, 0x0325, NULL, NULL, NULL, "IBM", "x3455", board_ibm_x3455}, + {0x1039, 0x5513, 0x8086, 0xd61f, 0x1039, 0x6330, 0x8086, 0xd61f, NULL, NULL, NULL, "Intel", "D201GLY", wbsio_check_for_spi}, + {0x1022, 0x7468, 0, 0, 0, 0, 0, 0, NULL, "iwill", "dk8_htx", "IWILL", "DK8-HTX", w83627hf_gpio24_raise_2e}, + {0x8086, 0x27A0, 0, 0, 0x8086, 0x27b8, 0, 0, NULL, "kontron", "986lcd-m", "Kontron", "986LCD-M", board_kontron_986lcd_m}, + {0x8086, 0x2411, 0x8086, 0x2411, 0x8086, 0x7125, 0x0e11, 0xb165, NULL, NULL, NULL, "Mitac", "6513WU", board_mitac_6513wu}, + {0x13f6, 0x0111, 0x1462, 0x5900, 0x1106, 0x3177, 0x1106, 0, NULL, NULL, NULL, "MSI", "MS-6590 (KT4 Ultra)",board_msi_kt4v}, + {0x1106, 0x3149, 0x1462, 0x7094, 0x10ec, 0x8167, 0x1462, 0x094c, NULL, NULL, NULL, "MSI", "MS-6702E (K8T Neo2-F)",w83627thf_gpio4_4_raise_2e}, + {0x1106, 0x0571, 0x1462, 0x7120, 0x1106, 0x3065, 0x1462, 0x7120, NULL, NULL, NULL, "MSI", "MS-6712 (KT4V)", board_msi_kt4v}, + {0x8086, 0x2658, 0x1462, 0x7046, 0x1106, 0x3044, 0x1462, 0x046d, NULL, NULL, NULL, "MSI", "MS-7046", intel_ich_gpio19_raise}, + {0x10DE, 0x005E, 0x1462, 0x7135, 0x10DE, 0x0050, 0x1462, 0x7135, NULL, "msi", "k8n-neo3", "MSI", "MS-7135 (K8N Neo3)", w83627thf_gpio4_4_raise_4e}, + {0x10DE, 0x005E, 0x1462, 0x7125, 0x10DE, 0x0052, 0x1462, 0x7125, NULL, NULL, NULL, "MSI", "K8N Neo4-F", nvidia_mcp_gpio2_raise}, + {0x1106, 0x3099, 0, 0, 0x1106, 0x3074, 0, 0, NULL, "shuttle", "ak31", "Shuttle", "AK31", w836xx_memw_enable_2e}, + {0x1106, 0x3104, 0x1297, 0xa238, 0x1106, 0x3059, 0x1297, 0xc063, NULL, NULL, NULL, "Shuttle", "AK38N", shuttle_ak38n}, + {0x10DE, 0x0050, 0x1297, 0x5036, 0x1412, 0x1724, 0x1297, 0x5036, NULL, NULL, NULL, "Shuttle", "FN25", board_shuttle_fn25}, + {0x1106, 0x3038, 0x0925, 0x1234, 0x1106, 0x3058, 0x15DD, 0x7609, NULL, NULL, NULL, "Soyo", "SY-7VCA", board_soyo_sy_7vca}, + {0x8086, 0x1076, 0x8086, 0x1176, 0x1106, 0x3059, 0x10f1, 0x2498, NULL, NULL, NULL, "Tyan", "S2498 (Tomcat K7M)", w836xx_memw_enable_2e}, + {0x1106, 0x3177, 0x1106, 0xAA01, 0x1106, 0x3123, 0x1106, 0xAA01, NULL, NULL, NULL, "VIA", "EPIA M/MII/...", via_vt823x_gpio15_raise}, + {0x1106, 0x0259, 0x1106, 0x3227, 0x1106, 0x3065, 0x1106, 0x3149, NULL, NULL, NULL, "VIA", "EPIA-N/NL", via_vt823x_gpio9_raise}, + {0x1106, 0x5337, 0x1458, 0xb003, 0x1106, 0x287e, 0x1106, 0x337e, NULL, NULL, NULL, "VIA", "PC3500G", it87xx_probe_spi_flash},
{ 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL, NULL}, /* end marker */ }; @@ -1314,6 +1319,18 @@ } }
+ if (board->dmi_identifier) { + if (!has_dmi_support) { + fprintf(stderr, "WARNING: Can't autodetect %s %s," + " DMI info unavailable.\n", + board->vendor_name, board->board_name); + continue; + } else { + if (!dmi_match(board->dmi_identifier)) + continue; + } + } + return board; }
Hi Michael,
thanks for your patch. Overall, I like it, but some design decisions may need to be revisited.
On 30.12.2009 15:02, Michael Karcher wrote:
Matching via DMI: If a board is not uniquely identifiable by PCI device/subsystem IDs, a string can be specified to be looked for (case-sensitive, substring) for now in one of the following items:
- System Manufacturer
- System Product Name
- System Version
- Baseboard Manufacturer
- Baseboard Product Name
- Baseboard Version
I believe the substring match is suboptimal. For example, if you want to differentiate between the Asus M2A-VM and the Asus M2A-VM HDMI, only the HDMI variant can be matched safely.
This may be a stupid question, but how do AWDFLASH etc. match their boards? Their mechanism is designed to work well under most circumstances, and the manufacturers check it for ambiguities.
The match is only made if DMI info is available and the string matches. If no DMI info is available, a warning is printed as the board can not be autodetected.
I'd make that warning conditional on the absence of a coreboot table. Otherwise you'll get a warning on a lot of systems with coreboot.
It's still open to discussion whether we add an DMI override switch to specify a string that will definitely match, and whether this switch is only used if no DMI is available or whether it overrides or augments DMI data.
I think DMI should be used as additional matching help if PCI IDs are already correct. Especially in case multiple vendors have the same Product Name string, the PCI ID matching is crucial.
DMI data is currently read using dmidecode. This tool is available for all major platforms except MacOS X. I heard that there also is a MacOS X version of dmidecode, but didn't investigate that.
We might want to specify a dmidecode path/binary as optional internal programmer parameter for installations which don't have a native dmidecode in their path. Maybe something like flashrom -p internal:dmidecode=c:\cygwin\bin\dmidecode.exe That's something I'd leave to a followup patch, though.
Signed-Off-By: Michael Karcher flashrom@mkarcher.dialup.fu-berlin.de
Index: dmi.c
--- dmi.c (Revision 0) +++ dmi.c (Revision 0) @@ -0,0 +1,82 @@
+enum dmi_strings {DMI_SYS_VENDOR, DMI_SYS_PRODUCT, DMI_SYS_VERSION,
DMI_BB_VENDOR, DMI_BB_PRODUCT, DMI_BB_VERSION,DMI_MAX_ID = DMI_BB_VERSION};
One line per value, please.
+const char * const dmidecode_names[DMI_MAX_ID+1] =
- {"system-manufacturer", "system-product-name",
"system-version", "baseboard-manufacturer","baseboard-product-name", "baseboard-version"};
Same here.
+int has_dmi_support = 0; +char dmistrings[DMI_MAX_ID+1][80];
+void dmi_init(void) +{
- FILE * dmidecode_pipe;
FILE *dmidecode_pipe;
Regards, Carl-Daniel
On 31.12.2009 17:22, Carl-Daniel Hailfinger wrote:
On 30.12.2009 15:02, Michael Karcher wrote:
Matching via DMI: If a board is not uniquely identifiable by PCI device/subsystem IDs, a string can be specified to be looked for (case-sensitive, substring) for now in one of the following items:
- System Manufacturer
- System Product Name
- System Version
- Baseboard Manufacturer
- Baseboard Product Name
- Baseboard Version
I believe the substring match is suboptimal. For example, if you want to differentiate between the Asus M2A-VM and the Asus M2A-VM HDMI, only the HDMI variant can be matched safely.
This may be a stupid question, but how do AWDFLASH etc. match their boards? Their mechanism is designed to work well under most circumstances, and the manufacturers check it for ambiguities.
If AWDFLASH etc. have no common scheme, I think we can come up with our own. Suggestion: - Ability to specify arbitrary combinations of all six strings without introducing a boatload of new fields. - (related to the point above) Storage format for string combinations, delimited with "," and starting with a field specifying the string selection (one char per selected string), delimiter, first string [, delimiter, second string [, delimiter, third string]]. The end result is still a human-readable string and can thus be stored in a simple C string. - Strip all leading and trailing whitespace from the strings before matching. Something like ltrim(rtrim($variable)) in BASIC. - Always match full strings (no substring matching).
Regards, Carl-Daniel
Am Freitag, den 01.01.2010, 01:18 +0100 schrieb Carl-Daniel Hailfinger:
If AWDFLASH etc. have no common scheme, I think we can come up with our own. Suggestion:
- Ability to specify arbitrary combinations of all six strings without
introducing a boatload of new fields.
- (related to the point above) Storage format for string combinations,
delimited with "," and starting with a field specifying the string selection (one char per selected string), delimiter, first string [, delimiter, second string [, delimiter, third string]]. The end result is still a human-readable string and can thus be stored in a simple C string.
You mean something like "mv,HP,VL400" for "system manufacturer HP, system version VL400". Yes, this is possible, but will break if the separator is included in the fields you want to match. But most important: Don't overdesign this stuff. This is only used to tell boards apart that already passed the PCI ID check.
- Strip all leading and trailing whitespace from the strings before
matching. Something like ltrim(rtrim($variable)) in BASIC.
Sounds sensible, but why? Leading/trailing space in DMI seems quite uncommon, and could be included in the expected pattern as well.
- Always match full strings (no substring matching).
This will break on Luc's HP example, IIRC. If we already go the route of implementing a parser for some (maybe trivial) matching language, one could at a asterisk in the beginning/end for non-anchored matches. It's still the question whether it really is worth the trouble.
Regards, Michael Karcher
^Matching via DMI: If a board is not uniquely identifiable by PCI device/subsystem IDs, a string can be specified to be looked for (case-sensitive, substring or anchored) for now in one of the following DMI items in addition to matching the PCI IDs: - System Manufacturer - System Product Name - System Version - Baseboard Manufacturer - Baseboard Product Name - Baseboard Version
Strings are anchored re-like (^ at the beginning, $ at the end), but there are no plans to support full regular expressions. Field selection has been intentionally omitted as the match via PCI IDs should be strong enough that no cross-pollution between the different strings should ever occur, as long as the string is meaningfull.
The match is only made if DMI info is available and the string matches. If no DMI info is available and the PCI IDs match, a warning is printed as the board can not be autodetected.
It's still open to discussion whether we add an DMI override switch to specify a string that will definitely match, and whether this switch is only used if no DMI is available or whether it overrides or augments DMI data.
DMI data is currently read using dmidecode. This tool is available for all major platforms except MacOS X. I heard that there also is a MacOS X version of dmidecode, but didn't investigate that.
Signed-Off-By: Michael Karcher flashrom@mkarcher.dialup.fu-berlin.de --- Revision 2 of this patch: - adjust coding style (one item per line, FILE *dmidecode_pipe, space after if, for, while) [carldani, comparing to other flashrom code] - Implement anchoring of the string [inspired by carldani] - use snprintf [agaran] - dmidecode path in separate constant [inspired by carldani]
Index: flash.h =================================================================== --- flash.h (Revision 824) +++ flash.h (Arbeitskopie) @@ -251,6 +251,9 @@ uint16_t second_card_vendor; uint16_t second_card_device;
+ /* String to appear in the DMI info */ + const char *dmi_identifier; + /* The vendor / part name from the coreboot table. */ const char *lb_vendor; const char *lb_part; @@ -330,6 +333,11 @@ extern char *lb_part, *lb_vendor; extern int partvendor_from_cbtable;
+/* dmi.c */ +extern int has_dmi_support; +void dmi_init(void); +int dmi_match(const char * identifier); + /* internal.c */ #if NEED_PCI == 1 struct superio { Index: Makefile =================================================================== --- Makefile (Revision 824) +++ Makefile (Arbeitskopie) @@ -101,7 +101,7 @@
ifeq ($(CONFIG_INTERNAL), yes) FEATURE_CFLAGS += -D'INTERNAL_SUPPORT=1' -PROGRAMMER_OBJS += chipset_enable.o board_enable.o cbtable.o it87spi.o ichspi.o sb600spi.o wbsio_spi.o +PROGRAMMER_OBJS += chipset_enable.o board_enable.o cbtable.o dmi.o it87spi.o ichspi.o sb600spi.o wbsio_spi.o NEED_PCI := yes endif
Index: dmi.c =================================================================== --- dmi.c (Revision 0) +++ dmi.c (Revision 0) @@ -0,0 +1,142 @@ +/* + * This file is part of the flashrom project. + * + * Copyright (C) 2009 Michael Karcher + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include <string.h> +#include <stdio.h> + +#include "flash.h" + +enum dmi_strings { + DMI_SYS_VENDOR, + DMI_SYS_PRODUCT, + DMI_SYS_VERSION, + DMI_BB_VENDOR, + DMI_BB_PRODUCT, + DMI_BB_VERSION, + DMI_ID_INVALID /* This must always be the last entry */ +}; + +const char * const dmidecode_names[DMI_ID_INVALID] = { + "system-manufacturer", + "system-product-name", + "system-version", + "baseboard-manufacturer", + "baseboard-product-name", + "baseboard-version" +}; + +#define DMI_COMMAND_LEN_MAX 260 +const char * dmidecode_command = "dmidecode"; + +int has_dmi_support = 0; +char dmistrings[DMI_ID_INVALID][80]; + +void dmi_init(void) +{ + FILE *dmidecode_pipe; + int i; + for (i = 0; i < DMI_ID_INVALID; i++) + { + char commandline[DMI_COMMAND_LEN_MAX+40]; + snprintf(commandline, sizeof(commandline), + "%s -s %s", dmidecode_command, dmidecode_names[i]); + dmidecode_pipe = popen(commandline, "r"); + fgets(dmistrings[i], sizeof(dmistrings[i]), dmidecode_pipe); + if (ferror(dmidecode_pipe)) + { + printf_debug("DMI pipe read error"); + pclose(dmidecode_pipe); + return; + } + /* Toss all output above 80 characters away to prevent + deadlock on pclose. */ + while (!feof(dmidecode_pipe)) + getc(dmidecode_pipe); + if (pclose(dmidecode_pipe) != 0) + { + printf_debug("DMI pipe close error"); + return; + } + + /* chomp trailing newline */ + if (dmistrings[i][0] != 0 && + dmistrings[i][strlen(dmistrings[i]) - 1] == '\n') + dmistrings[i][strlen(dmistrings[i]) - 1] = 0; + printf_debug("DMI string %d: %s\n", i, dmistrings[i]); + } + has_dmi_support = 1; +} + +/** + * Does an substring/prefix/postfix/whole-string match. + * + * The pattern is matched as-is. The only metacharacters supported are '^' + * at the beginning and '$' at the end. So you can look for "^prefix", + * "suffix$", "substring" or "^complete string$". + * + * @param value The string to check. + * @param pattern The pattern. + * @return Nonzero if pattern matches. + */ +static int dmi_compare(const char * value, const char * pattern) +{ + int anchored = 0; + int patternlen; + /* The empty string is part of all strings */ + if (pattern[0] == 0) + return 1; + + if (pattern[0] == '^') { + anchored = 1; + pattern++; + } + + patternlen = strlen(pattern); + if (pattern[patternlen - 1] == '$') { + int valuelen = strlen(value); + patternlen--; + if(patternlen > valuelen) + return 0; + + /* full string match: require same length */ + if(anchored && valuelen != patternlen) + return 0; + + /* start character to make ends match */ + value += valuelen - patternlen; + anchored = 1; + } + + if (anchored) + return strncmp(value, pattern, patternlen) == 0; + else + return strstr(value, pattern) != NULL; +} + +int dmi_match(const char * identifier) +{ + int i; + if (!has_dmi_support) + return 0; + for (i = 0;i < DMI_ID_INVALID; i++) + if (dmi_compare(dmistrings[i], identifier)) + return 1; + return 0; +} Index: internal.c =================================================================== --- internal.c (Revision 824) +++ internal.c (Arbeitskopie) @@ -152,6 +152,7 @@ * mainboard specific flash enable sequence. */ coreboot_init(); + dmi_init();
/* Probe for the SuperI/O chip and fill global struct superio. */ probe_superio(); Index: board_enable.c =================================================================== --- board_enable.c (Revision 824) +++ board_enable.c (Arbeitskopie) @@ -1162,6 +1162,11 @@ * provide an as complete set of pci ids as possible; autodetection is the * preferred behaviour and we would like to make sure that matches are unique. * + * If PCI IDs are not sufficient for board matching, the match can be further + * constrained by a string that has to be present in the DMI database for + * the baseboard or the system entry. The match type is case-sensitive + * substring match. + * * The coreboot ids are used two fold. When running with a coreboot firmware, * the ids uniquely matches the coreboot board identification string. When a * legacy bios is installed and when autodetection is not possible, these ids @@ -1173,61 +1178,61 @@
/* Please keep this list alphabetically ordered by vendor/board name. */ struct board_pciid_enable board_pciid_enables[] = { - /* first pci-id set [4], second pci-id set [4], coreboot id [2], vendor name board name flash enable */ - {0x8086, 0x2926, 0x147b, 0x1084, 0x11ab, 0x4364, 0x147b, 0x1084, NULL, NULL, "Abit", "IP35", intel_ich_gpio16_raise}, - {0x105a, 0x0d30, 0x105a, 0x4d33, 0x8086, 0x1130, 0x8086, 0, NULL, NULL, "Acorp", "6A815EPD", board_acorp_6a815epd}, - {0x8086, 0x24D4, 0x1849, 0x24D0, 0x8086, 0x24D5, 0x1849, 0x9739, NULL, NULL, "ASRock", "P4i65GV", intel_ich_gpio23_raise}, - {0x1022, 0x746B, 0, 0, 0, 0, 0, 0, "AGAMI", "ARUMA", "agami", "Aruma", w83627hf_gpio24_raise_2e}, - {0x1106, 0x3177, 0x17F2, 0x3177, 0x1106, 0x3148, 0x17F2, 0x3148, NULL, NULL, "Albatron", "PM266A", w836xx_memw_enable_2e}, - {0x1106, 0x3205, 0x1106, 0x3205, 0x10EC, 0x8139, 0xA0A0, 0x0477, NULL, NULL, "AOpen", "vKM400Am-S", it8705_rom_write_enable}, - {0x1022, 0x2090, 0, 0, 0x1022, 0x2080, 0, 0, "artecgroup", "dbe61", "Artec Group", "DBE61", board_artecgroup_dbe6x}, - {0x1022, 0x2090, 0, 0, 0x1022, 0x2080, 0, 0, "artecgroup", "dbe62", "Artec Group", "DBE62", board_artecgroup_dbe6x}, - {0x1106, 0x3189, 0x1043, 0x807F, 0x1106, 0x3065, 0x1043, 0x80ED, NULL, NULL, "ASUS", "A7V600-X", board_asus_a7v600x}, - {0x1106, 0x3189, 0x1043, 0x807F, 0x1106, 0x3177, 0x1043, 0x808C, NULL, NULL, "ASUS", "A7V8X", board_asus_a7v8x}, - {0x1106, 0x3177, 0x1043, 0x80A1, 0x1106, 0x3205, 0x1043, 0x8118, NULL, NULL, "ASUS", "A7V8X-MX SE", w836xx_memw_enable_2e}, - {0x1106, 0x1336, 0x1043, 0x80ed, 0x1106, 0x3288, 0x1043, 0x8249, NULL, NULL, "ASUS", "M2V-MX", via_vt823x_gpio5_raise}, - {0x8086, 0x1a30, 0x1043, 0x8070, 0x8086, 0x244b, 0x1043, 0x8028, NULL, NULL, "ASUS", "P4B266", intel_ich_gpio22_raise}, - {0x8086, 0x1A30, 0x1043, 0x8025, 0x8086, 0x244B, 0x104D, 0x80F0, NULL, NULL, "ASUS", "P4B266-LM", intel_ich_gpio21_raise}, - {0x8086, 0x2570, 0x1043, 0x80F2, 0x105A, 0x3373, 0x1043, 0x80F5, NULL, NULL, "ASUS", "P4P800-E Deluxe", intel_ich_gpio21_raise}, - {0x10B9, 0x1541, 0, 0, 0x10B9, 0x1533, 0, 0, "asus", "p5a", "ASUS", "P5A", board_asus_p5a}, - {0x10DE, 0x0030, 0x1043, 0x818a, 0x8086, 0x100E, 0x1043, 0x80EE, NULL, NULL, "ASUS", "P5ND2-SLI Deluxe", nvidia_mcp_gpio10_raise}, - {0x1106, 0x3149, 0x1565, 0x3206, 0x1106, 0x3344, 0x1565, 0x1202, NULL, NULL, "Biostar", "P4M80-M4", it8705_rom_write_enable}, - {0x8086, 0x3590, 0x1028, 0x016c, 0x1000, 0x0030, 0x1028, 0x016c, NULL, NULL, "Dell", "PowerEdge 1850", intel_ich_gpio23_raise}, - {0x1039, 0x5513, 0x1019, 0x0A41, 0x1039, 0x0018, 0, 0, NULL, NULL, "Elitegroup", "K7S6A", elitegroup_k7vta3}, - {0x1106, 0x3038, 0x1019, 0x0996, 0x1106, 0x3177, 0x1019, 0x0996, NULL, NULL, "Elitegroup", "K7VTA3", elitegroup_k7vta3}, - {0x1106, 0x3177, 0x1106, 0x3177, 0x1106, 0x3059, 0x1695, 0x3005, NULL, NULL, "EPoX", "EP-8K5A2", w836xx_memw_enable_2e}, - {0x10EC, 0x8139, 0x1695, 0x9001, 0x11C1, 0x5811, 0x1695, 0x9015, NULL, NULL, "EPoX", "EP-8RDA3+", nvidia_mcp_gpio31_raise}, - {0x8086, 0x7110, 0, 0, 0x8086, 0x7190, 0, 0, "epox", "ep-bx3", "EPoX", "EP-BX3", board_epox_ep_bx3}, - {0x1039, 0x0761, 0, 0, 0x10EC, 0x8168, 0, 0, "gigabyte", "2761gxdk", "GIGABYTE", "GA-2761GXDK", it87xx_probe_spi_flash}, - {0x1106, 0x3227, 0x1458, 0x5001, 0x10ec, 0x8139, 0x1458, 0xe000, NULL, NULL, "GIGABYTE", "GA-7VT600", it8705_rom_write_enable}, - {0x10DE, 0x0050, 0x1458, 0x0C11, 0x10DE, 0x005e, 0x1458, 0x5000, NULL, NULL, "GIGABYTE", "GA-K8N-SLI", nvidia_mcp_gpio21_raise}, - {0x10DE, 0x0360, 0x1458, 0x0C11, 0x10DE, 0x0369, 0x1458, 0x5001, "gigabyte", "m57sli", "GIGABYTE", "GA-M57SLI-S4", it87xx_probe_spi_flash}, - {0x10de, 0x03e0, 0, 0, 0x10DE, 0x03D0, 0, 0, NULL, NULL, "GIGABYTE", "GA-M61P-S3", it87xx_probe_spi_flash}, - {0x1002, 0x4398, 0x1458, 0x5004, 0x1002, 0x4391, 0x1458, 0xb000, NULL, NULL, "GIGABYTE", "GA-MA78G-DS3H", it87xx_probe_spi_flash}, - {0x1002, 0x4398, 0x1458, 0x5004, 0x1002, 0x4391, 0x1458, 0xb002, NULL, NULL, "GIGABYTE", "GA-MA78GM-S2H", it87xx_probe_spi_flash}, - {0x1002, 0x438d, 0x1458, 0x5001, 0x1002, 0x5956, 0x1002, 0x5956, NULL, NULL, "GIGABYTE", "GA-MA790FX-DQ6", it87xx_probe_spi_flash}, - {0x1166, 0x0223, 0x103c, 0x320d, 0x102b, 0x0522, 0x103c, 0x31fa, "hp", "dl145_g3", "HP", "DL145 G3", board_hp_dl145_g3_enable}, - {0x1166, 0x0205, 0x1014, 0x0347, 0x1002, 0x515E, 0x1014, 0x0325, NULL, NULL, "IBM", "x3455", board_ibm_x3455}, - {0x1039, 0x5513, 0x8086, 0xd61f, 0x1039, 0x6330, 0x8086, 0xd61f, NULL, NULL, "Intel", "D201GLY", wbsio_check_for_spi}, - {0x1022, 0x7468, 0, 0, 0, 0, 0, 0, "iwill", "dk8_htx", "IWILL", "DK8-HTX", w83627hf_gpio24_raise_2e}, - {0x8086, 0x27A0, 0, 0, 0x8086, 0x27b8, 0, 0, "kontron", "986lcd-m", "Kontron", "986LCD-M", board_kontron_986lcd_m}, - {0x8086, 0x2411, 0x8086, 0x2411, 0x8086, 0x7125, 0x0e11, 0xb165, NULL, NULL, "Mitac", "6513WU", board_mitac_6513wu}, - {0x13f6, 0x0111, 0x1462, 0x5900, 0x1106, 0x3177, 0x1106, 0, NULL, NULL, "MSI", "MS-6590 (KT4 Ultra)",board_msi_kt4v}, - {0x1106, 0x3149, 0x1462, 0x7094, 0x10ec, 0x8167, 0x1462, 0x094c, NULL, NULL, "MSI", "MS-6702E (K8T Neo2-F)",w83627thf_gpio4_4_raise_2e}, - {0x1106, 0x0571, 0x1462, 0x7120, 0x1106, 0x3065, 0x1462, 0x7120, NULL, NULL, "MSI", "MS-6712 (KT4V)", board_msi_kt4v}, - {0x8086, 0x2658, 0x1462, 0x7046, 0x1106, 0x3044, 0x1462, 0x046d, NULL, NULL, "MSI", "MS-7046", intel_ich_gpio19_raise}, - {0x10DE, 0x005E, 0x1462, 0x7135, 0x10DE, 0x0050, 0x1462, 0x7135, "msi", "k8n-neo3", "MSI", "MS-7135 (K8N Neo3)", w83627thf_gpio4_4_raise_4e}, - {0x10DE, 0x005E, 0x1462, 0x7125, 0x10DE, 0x0052, 0x1462, 0x7125, NULL, NULL, "MSI", "K8N Neo4-F", nvidia_mcp_gpio2_raise}, - {0x1106, 0x3099, 0, 0, 0x1106, 0x3074, 0, 0, "shuttle", "ak31", "Shuttle", "AK31", w836xx_memw_enable_2e}, - {0x1106, 0x3104, 0x1297, 0xa238, 0x1106, 0x3059, 0x1297, 0xc063, NULL, NULL, "Shuttle", "AK38N", shuttle_ak38n}, - {0x10DE, 0x0050, 0x1297, 0x5036, 0x1412, 0x1724, 0x1297, 0x5036, NULL, NULL, "Shuttle", "FN25", board_shuttle_fn25}, - {0x1106, 0x3038, 0x0925, 0x1234, 0x1106, 0x3058, 0x15DD, 0x7609, NULL, NULL, "Soyo", "SY-7VCA", board_soyo_sy_7vca}, - {0x8086, 0x1076, 0x8086, 0x1176, 0x1106, 0x3059, 0x10f1, 0x2498, NULL, NULL, "Tyan", "S2498 (Tomcat K7M)", w836xx_memw_enable_2e}, - {0x1106, 0x3177, 0x1106, 0xAA01, 0x1106, 0x3123, 0x1106, 0xAA01, NULL, NULL, "VIA", "EPIA M/MII/...", via_vt823x_gpio15_raise}, - {0x1106, 0x0259, 0x1106, 0x3227, 0x1106, 0x3065, 0x1106, 0x3149, NULL, NULL, "VIA", "EPIA-N/NL", via_vt823x_gpio9_raise}, - {0x1106, 0x5337, 0x1458, 0xb003, 0x1106, 0x287e, 0x1106, 0x337e, NULL, NULL, "VIA", "PC3500G", it87xx_probe_spi_flash}, + /* first pci-id set [4], second pci-id set [4], dmi_identifier, coreboot id [2], vendor name board name flash enable */ + {0x8086, 0x2926, 0x147b, 0x1084, 0x11ab, 0x4364, 0x147b, 0x1084, NULL, NULL, NULL, "Abit", "IP35", intel_ich_gpio16_raise}, + {0x105a, 0x0d30, 0x105a, 0x4d33, 0x8086, 0x1130, 0x8086, 0, NULL, NULL, NULL, "Acorp", "6A815EPD", board_acorp_6a815epd}, + {0x8086, 0x24D4, 0x1849, 0x24D0, 0x8086, 0x24D5, 0x1849, 0x9739, NULL, NULL, NULL, "ASRock", "P4i65GV", intel_ich_gpio23_raise}, + {0x1022, 0x746B, 0, 0, 0, 0, 0, 0, NULL, "AGAMI", "ARUMA", "agami", "Aruma", w83627hf_gpio24_raise_2e}, + {0x1106, 0x3177, 0x17F2, 0x3177, 0x1106, 0x3148, 0x17F2, 0x3148, NULL, NULL, NULL, "Albatron", "PM266A", w836xx_memw_enable_2e}, + {0x1106, 0x3205, 0x1106, 0x3205, 0x10EC, 0x8139, 0xA0A0, 0x0477, NULL, NULL, NULL, "AOpen", "vKM400Am-S", it8705_rom_write_enable}, + {0x1022, 0x2090, 0, 0, 0x1022, 0x2080, 0, 0, NULL, "artecgroup", "dbe61", "Artec Group", "DBE61", board_artecgroup_dbe6x}, + {0x1022, 0x2090, 0, 0, 0x1022, 0x2080, 0, 0, NULL, "artecgroup", "dbe62", "Artec Group", "DBE62", board_artecgroup_dbe6x}, + {0x1106, 0x3189, 0x1043, 0x807F, 0x1106, 0x3065, 0x1043, 0x80ED, NULL, NULL, NULL, "ASUS", "A7V600-X", board_asus_a7v600x}, + {0x1106, 0x3189, 0x1043, 0x807F, 0x1106, 0x3177, 0x1043, 0x808C, NULL, NULL, NULL, "ASUS", "A7V8X", board_asus_a7v8x}, + {0x1106, 0x3177, 0x1043, 0x80A1, 0x1106, 0x3205, 0x1043, 0x8118, NULL, NULL, NULL, "ASUS", "A7V8X-MX SE", w836xx_memw_enable_2e}, + {0x1106, 0x1336, 0x1043, 0x80ed, 0x1106, 0x3288, 0x1043, 0x8249, NULL, NULL, NULL, "ASUS", "M2V-MX", via_vt823x_gpio5_raise}, + {0x8086, 0x1a30, 0x1043, 0x8070, 0x8086, 0x244b, 0x1043, 0x8028, NULL, NULL, NULL, "ASUS", "P4B266", intel_ich_gpio22_raise}, + {0x8086, 0x1A30, 0x1043, 0x8025, 0x8086, 0x244B, 0x104D, 0x80F0, NULL, NULL, NULL, "ASUS", "P4B266-LM", intel_ich_gpio21_raise}, + {0x8086, 0x2570, 0x1043, 0x80F2, 0x105A, 0x3373, 0x1043, 0x80F5, NULL, NULL, NULL, "ASUS", "P4P800-E Deluxe", intel_ich_gpio21_raise}, + {0x10B9, 0x1541, 0, 0, 0x10B9, 0x1533, 0, 0, NULL, "asus", "p5a", "ASUS", "P5A", board_asus_p5a}, + {0x10DE, 0x0030, 0x1043, 0x818a, 0x8086, 0x100E, 0x1043, 0x80EE, NULL, NULL, NULL, "ASUS", "P5ND2-SLI Deluxe", nvidia_mcp_gpio10_raise}, + {0x1106, 0x3149, 0x1565, 0x3206, 0x1106, 0x3344, 0x1565, 0x1202, NULL, NULL, NULL, "Biostar", "P4M80-M4", it8705_rom_write_enable}, + {0x8086, 0x3590, 0x1028, 0x016c, 0x1000, 0x0030, 0x1028, 0x016c, NULL, NULL, NULL, "Dell", "PowerEdge 1850", intel_ich_gpio23_raise}, + {0x1039, 0x5513, 0x1019, 0x0A41, 0x1039, 0x0018, 0, 0, NULL, NULL, NULL, "Elitegroup", "K7S6A", elitegroup_k7vta3}, + {0x1106, 0x3038, 0x1019, 0x0996, 0x1106, 0x3177, 0x1019, 0x0996, NULL, NULL, NULL, "Elitegroup", "K7VTA3", elitegroup_k7vta3}, + {0x1106, 0x3177, 0x1106, 0x3177, 0x1106, 0x3059, 0x1695, 0x3005, NULL, NULL, NULL, "EPoX", "EP-8K5A2", w836xx_memw_enable_2e}, + {0x10EC, 0x8139, 0x1695, 0x9001, 0x11C1, 0x5811, 0x1695, 0x9015, NULL, NULL, NULL, "EPoX", "EP-8RDA3+", nvidia_mcp_gpio31_raise}, + {0x8086, 0x7110, 0, 0, 0x8086, 0x7190, 0, 0, NULL, "epox", "ep-bx3", "EPoX", "EP-BX3", board_epox_ep_bx3}, + {0x1039, 0x0761, 0, 0, 0x10EC, 0x8168, 0, 0, NULL, "gigabyte", "2761gxdk", "GIGABYTE", "GA-2761GXDK", it87xx_probe_spi_flash}, + {0x1106, 0x3227, 0x1458, 0x5001, 0x10ec, 0x8139, 0x1458, 0xe000, NULL, NULL, NULL, "GIGABYTE", "GA-7VT600", it8705_rom_write_enable}, + {0x10DE, 0x0050, 0x1458, 0x0C11, 0x10DE, 0x005e, 0x1458, 0x5000, NULL, NULL, NULL, "GIGABYTE", "GA-K8N-SLI", nvidia_mcp_gpio21_raise}, + {0x10DE, 0x0360, 0x1458, 0x0C11, 0x10DE, 0x0369, 0x1458, 0x5001, NULL, "gigabyte", "m57sli", "GIGABYTE", "GA-M57SLI-S4", it87xx_probe_spi_flash}, + {0x10de, 0x03e0, 0, 0, 0x10DE, 0x03D0, 0, 0, NULL, NULL, NULL, "GIGABYTE", "GA-M61P-S3", it87xx_probe_spi_flash}, + {0x1002, 0x4398, 0x1458, 0x5004, 0x1002, 0x4391, 0x1458, 0xb000, NULL, NULL, NULL, "GIGABYTE", "GA-MA78G-DS3H", it87xx_probe_spi_flash}, + {0x1002, 0x4398, 0x1458, 0x5004, 0x1002, 0x4391, 0x1458, 0xb002, NULL, NULL, NULL, "GIGABYTE", "GA-MA78GM-S2H", it87xx_probe_spi_flash}, + {0x1002, 0x438d, 0x1458, 0x5001, 0x1002, 0x5956, 0x1002, 0x5956, NULL, NULL, NULL, "GIGABYTE", "GA-MA790FX-DQ6", it87xx_probe_spi_flash}, + {0x1166, 0x0223, 0x103c, 0x320d, 0x102b, 0x0522, 0x103c, 0x31fa, NULL, "hp", "dl145_g3", "HP", "DL145 G3", board_hp_dl145_g3_enable}, + {0x1166, 0x0205, 0x1014, 0x0347, 0x1002, 0x515E, 0x1014, 0x0325, NULL, NULL, NULL, "IBM", "x3455", board_ibm_x3455}, + {0x1039, 0x5513, 0x8086, 0xd61f, 0x1039, 0x6330, 0x8086, 0xd61f, NULL, NULL, NULL, "Intel", "D201GLY", wbsio_check_for_spi}, + {0x1022, 0x7468, 0, 0, 0, 0, 0, 0, NULL, "iwill", "dk8_htx", "IWILL", "DK8-HTX", w83627hf_gpio24_raise_2e}, + {0x8086, 0x27A0, 0, 0, 0x8086, 0x27b8, 0, 0, NULL, "kontron", "986lcd-m", "Kontron", "986LCD-M", board_kontron_986lcd_m}, + {0x8086, 0x2411, 0x8086, 0x2411, 0x8086, 0x7125, 0x0e11, 0xb165, NULL, NULL, NULL, "Mitac", "6513WU", board_mitac_6513wu}, + {0x13f6, 0x0111, 0x1462, 0x5900, 0x1106, 0x3177, 0x1106, 0, NULL, NULL, NULL, "MSI", "MS-6590 (KT4 Ultra)",board_msi_kt4v}, + {0x1106, 0x3149, 0x1462, 0x7094, 0x10ec, 0x8167, 0x1462, 0x094c, NULL, NULL, NULL, "MSI", "MS-6702E (K8T Neo2-F)",w83627thf_gpio4_4_raise_2e}, + {0x1106, 0x0571, 0x1462, 0x7120, 0x1106, 0x3065, 0x1462, 0x7120, NULL, NULL, NULL, "MSI", "MS-6712 (KT4V)", board_msi_kt4v}, + {0x8086, 0x2658, 0x1462, 0x7046, 0x1106, 0x3044, 0x1462, 0x046d, NULL, NULL, NULL, "MSI", "MS-7046", intel_ich_gpio19_raise}, + {0x10DE, 0x005E, 0x1462, 0x7135, 0x10DE, 0x0050, 0x1462, 0x7135, NULL, "msi", "k8n-neo3", "MSI", "MS-7135 (K8N Neo3)", w83627thf_gpio4_4_raise_4e}, + {0x10DE, 0x005E, 0x1462, 0x7125, 0x10DE, 0x0052, 0x1462, 0x7125, NULL, NULL, NULL, "MSI", "K8N Neo4-F", nvidia_mcp_gpio2_raise}, + {0x1106, 0x3099, 0, 0, 0x1106, 0x3074, 0, 0, NULL, "shuttle", "ak31", "Shuttle", "AK31", w836xx_memw_enable_2e}, + {0x1106, 0x3104, 0x1297, 0xa238, 0x1106, 0x3059, 0x1297, 0xc063, NULL, NULL, NULL, "Shuttle", "AK38N", shuttle_ak38n}, + {0x10DE, 0x0050, 0x1297, 0x5036, 0x1412, 0x1724, 0x1297, 0x5036, NULL, NULL, NULL, "Shuttle", "FN25", board_shuttle_fn25}, + {0x1106, 0x3038, 0x0925, 0x1234, 0x1106, 0x3058, 0x15DD, 0x7609, NULL, NULL, NULL, "Soyo", "SY-7VCA", board_soyo_sy_7vca}, + {0x8086, 0x1076, 0x8086, 0x1176, 0x1106, 0x3059, 0x10f1, 0x2498, NULL, NULL, NULL, "Tyan", "S2498 (Tomcat K7M)", w836xx_memw_enable_2e}, + {0x1106, 0x3177, 0x1106, 0xAA01, 0x1106, 0x3123, 0x1106, 0xAA01, NULL, NULL, NULL, "VIA", "EPIA M/MII/...", via_vt823x_gpio15_raise}, + {0x1106, 0x0259, 0x1106, 0x3227, 0x1106, 0x3065, 0x1106, 0x3149, NULL, NULL, NULL, "VIA", "EPIA-N/NL", via_vt823x_gpio9_raise}, + {0x1106, 0x5337, 0x1458, 0xb003, 0x1106, 0x287e, 0x1106, 0x337e, NULL, NULL, NULL, "VIA", "PC3500G", it87xx_probe_spi_flash},
- { 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL, NULL}, /* end marker */ + { 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL, NULL, NULL}, /* end marker */ };
/** @@ -1314,6 +1319,18 @@ } }
+ if (board->dmi_identifier) { + if (!has_dmi_support) { + fprintf(stderr, "WARNING: Can't autodetect %s %s," + " DMI info unavailable.\n", + board->vendor_name, board->board_name); + continue; + } else { + if (!dmi_match(board->dmi_identifier)) + continue; + } + } + return board; }
On 03.01.2010 01:09, Michael Karcher wrote:
^Matching via DMI: If a board is not uniquely identifiable by PCI device/subsystem IDs, a string can be specified to be looked for (case-sensitive, substring or anchored) for now in one of the following DMI items in addition to matching the PCI IDs:
- System Manufacturer
- System Product Name
- System Version
- Baseboard Manufacturer
- Baseboard Product Name
- Baseboard Version
Strings are anchored re-like (^ at the beginning, $ at the end), but there are no plans to support full regular expressions. Field selection has been intentionally omitted as the match via PCI IDs should be strong enough that no cross-pollution between the different strings should ever occur, as long as the string is meaningfull.
The match is only made if DMI info is available and the string matches. If no DMI info is available and the PCI IDs match, a warning is printed as the board can not be autodetected.
It's still open to discussion whether we add an DMI override switch to specify a string that will definitely match, and whether this switch is only used if no DMI is available or whether it overrides or augments DMI data.
Hm. Good question. I'd leave that to future discussion, though.
DMI data is currently read using dmidecode. This tool is available for all major platforms except MacOS X. I heard that there also is a MacOS X version of dmidecode, but didn't investigate that.
Signed-Off-By: Michael Karcher flashrom@mkarcher.dialup.fu-berlin.de
Revision 2 of this patch:
- adjust coding style (one item per line, FILE *dmidecode_pipe, space
after if, for, while) [carldani, comparing to other flashrom code]
- Implement anchoring of the string [inspired by carldani]
- use snprintf [agaran]
- dmidecode path in separate constant [inspired by carldani]
Thanks. I stripped everything I agree with, the review below only has the code snippets where I have comments.
Index: dmi.c
--- dmi.c (Revision 0) +++ dmi.c (Revision 0) @@ -0,0 +1,142 @@ +/*
- This file is part of the flashrom project.
- Copyright (C) 2009 Michael Karcher
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License as published by
- the Free Software Foundation; either version 2 of the License, or
- (at your option) any later version.
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
- You should have received a copy of the GNU General Public License
- along with this program; if not, write to the Free Software
- Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
- */
+#include <string.h> +#include <stdio.h>
+#include "flash.h"
+enum dmi_strings {
DMI_SYS_VENDOR,DMI_SYS_PRODUCT,DMI_SYS_VERSION,DMI_BB_VENDOR,DMI_BB_PRODUCT,DMI_BB_VERSION,DMI_ID_INVALID /* This must always be the last entry */+};
Please use tabs instead of spaces for indentation (all enum values above).
+const char * const dmidecode_names[DMI_ID_INVALID] = {
"system-manufacturer","system-product-name",
Please use tabs instead of spaces for indentation (the two strings above).
- "system-version",
- "baseboard-manufacturer",
- "baseboard-product-name",
- "baseboard-version"
+};
+#define DMI_COMMAND_LEN_MAX 260 +const char * dmidecode_command = "dmidecode";
Remove space after *.
+int has_dmi_support = 0; +char dmistrings[DMI_ID_INVALID][80];
I'm not so happy about the static allocation here. Can't we use malloc() for the read buffer and strdup() for filling in dmistrings?
+void dmi_init(void) +{
- FILE *dmidecode_pipe;
- int i;
- for (i = 0; i < DMI_ID_INVALID; i++)
- {
char commandline[DMI_COMMAND_LEN_MAX+40];snprintf(commandline, sizeof(commandline),"%s -s %s", dmidecode_command, dmidecode_names[i]);dmidecode_pipe = popen(commandline, "r");
The popen() man page suggests fflush(NULL) before popen(). Not sure. I'm not seeing any error checking on popen() here.
fgets(dmistrings[i], sizeof(dmistrings[i]), dmidecode_pipe);
Hm. Check the result of fgets()?
if (ferror(dmidecode_pipe)){printf_debug("DMI pipe read error");pclose(dmidecode_pipe);return;}/* Toss all output above 80 characters away to preventdeadlock on pclose. */while (!feof(dmidecode_pipe))getc(dmidecode_pipe);if (pclose(dmidecode_pipe) != 0){printf_debug("DMI pipe close error");return;}/* chomp trailing newline */if (dmistrings[i][0] != 0 &&dmistrings[i][strlen(dmistrings[i]) - 1] == '\n')dmistrings[i][strlen(dmistrings[i]) - 1] = 0;
Please use tabs instead of spaces for indentation (line above).
printf_debug("DMI string %d: %s\n", i, dmistrings[i]);- }
- has_dmi_support = 1;
+}
+/**
- Does an substring/prefix/postfix/whole-string match.
- The pattern is matched as-is. The only metacharacters supported are '^'
- at the beginning and '$' at the end. So you can look for "^prefix",
- "suffix$", "substring" or "^complete string$".
- @param value The string to check.
- @param pattern The pattern.
- @return Nonzero if pattern matches.
- */
+static int dmi_compare(const char * value, const char * pattern)
Please remove the space after *.
+{
int anchored = 0;int patternlen;/* The empty string is part of all strings */if (pattern[0] == 0)return 1;if (pattern[0] == '^') {anchored = 1;pattern++;}patternlen = strlen(pattern);if (pattern[patternlen - 1] == '$') {int valuelen = strlen(value);patternlen--;if(patternlen > valuelen)return 0;/* full string match: require same length */if(anchored && valuelen != patternlen)
Additional () please. Precedence is nice, but readability is even better.
return 0;/* start character to make ends match */value += valuelen - patternlen;anchored = 1;}if (anchored)return strncmp(value, pattern, patternlen) == 0;elsereturn strstr(value, pattern) != NULL;+}
Please use tabs instead of spaces for indentation (whole function above).
+int dmi_match(const char * identifier) +{
- int i;
- if (!has_dmi_support)
return 0;- for (i = 0;i < DMI_ID_INVALID; i++)
if (dmi_compare(dmistrings[i], identifier))
I believe the "match any of the strings" policy is fundamentally wrong. If you and Luc think specifying multiple strings for the match is overkill/overdesign, please do at least allow specifying the DMI string this should be compared with. Since this is a policy decision, we could have the inner loop here inside #ifdef FIXME_POLICY_DECISION or whatever (resulting in a non-match for all inputs) to get the infrstructure merged, then finish the policy decision in a separate patch.
return 1;- return 0;
+} Index: board_enable.c =================================================================== --- board_enable.c (Revision 824) +++ board_enable.c (Arbeitskopie) @@ -1162,6 +1162,11 @@
- provide an as complete set of pci ids as possible; autodetection is the
- preferred behaviour and we would like to make sure that matches are unique.
- If PCI IDs are not sufficient for board matching, the match can be further
- constrained by a string that has to be present in the DMI database for
- the baseboard or the system entry. The match type is case-sensitive
- substring match.
Maybe add some text which tells people to always include DMI IDs if they create new board matches. Not sure. It would certainly tighten board matching, and we don't know how bad the subsystem ID overlap between different boards is.
- The coreboot ids are used two fold. When running with a coreboot firmware,
- the ids uniquely matches the coreboot board identification string. When a
- legacy bios is installed and when autodetection is not possible, these ids
@@ -1314,6 +1319,18 @@ } }
if (board->dmi_identifier) {if (!has_dmi_support) {fprintf(stderr, "WARNING: Can't autodetect %s %s,"" DMI info unavailable.\n",board->vendor_name, board->board_name);continue;} else {if (!dmi_match(board->dmi_identifier))continue;}}
Some of the indentation in the block above uses spaces instead of tabs. Spaces are only OK if they are less than 8 and if they are preceded by tabs. The most common use case for spaces is indenting function arguments or expressions.
One more iteration, and I think you can commit.
Regards, Carl-Daniel
Hello Carl-Daniel,
thanks for your review.
There seems to be a fundamental opinion difference what DMI is used for between you on the one side and me and Luc on the other side. You seem to want to use DMI for a good board identification (which is fair enough, I also started with that intention), whereas now we decided to use DMI only as an emergency helper in case the PCI IDs seem obviously not good enough, which should be the exception instead of the regular case.
Using DMI as emergency rescue has the advantage that it still works on most systems even if DMI is unavailable (boards having DMI info can not be matched by PCI IDs with the matching policy of this patch). Collecting DMI info on boards with good PCI subsystem IDs (like the MSI boards that have the internal 4-digit product number BCD-coded in the subsystem IDs) is not useful, in my opinion.
The usage of DMI as breaker explains the loose matching policy of "any DMI string matches", as the IDs that should go into the table should be unique enough even with this weak policy (I for example can't imagine any HP system, as the manufacturer has already been established by subsystem manufacturer IDs, where "^VL420$" is going to match in the wrong string). If you want to use DMI as general board identification mechanism, independent of the PCI IDs, one would of course have to check more than one field.
Am Montag, den 04.01.2010, 04:53 +0100 schrieb Carl-Daniel Hailfinger: [...]
+int has_dmi_support = 0; +char dmistrings[DMI_ID_INVALID][80];
I'm not so happy about the static allocation here. Can't we use malloc() for the read buffer and strdup() for filling in dmistrings?
Of course we can. On the other hand, I don't really see the point in using dynamic allocation here. If it's about wasted memory, we should approach "struct flashchips" first. The fixed-size arrays in it waste a lot more. Nevertheless, I change it to dynamic allocation. Too bad the nice getline function that reads one line and allocates dynamically is GNU-only and not standard C or at least POSIX.
fgets(dmistrings[i], sizeof(dmistrings[i]), dmidecode_pipe);Hm. Check the result of fgets()?
Omitted by choice. The result of fgets is useless. It returns NULL if an error occurred (we would like to know) or if EOF occurred with no characters read (which is valid if the BIOS doesn't specify the string we are trying to read). So instead of checking the ambiguous return value of fgets, I check the ferror flag afterwards.
+int dmi_match(const char * identifier) +{
- int i;
- if (!has_dmi_support)
return 0;- for (i = 0;i < DMI_ID_INVALID; i++)
if (dmi_compare(dmistrings[i], identifier))I believe the "match any of the strings" policy is fundamentally wrong. If you and Luc think specifying multiple strings for the match is overkill/overdesign, please do at least allow specifying the DMI string this should be compared with.
Fair enough. I added it. Specifying one string to match is not increasing complexity much, although it will be a three-character hit on the board_enable table (if you specify like "bp:^A8V MX$" for baseboard product match for example). Luc, please NACK if you think the it is too much.
Since this is a policy decision, we could have the inner loop here inside #ifdef FIXME_POLICY_DECISION or whatever (resulting in a non-match for all inputs) to get the infrstructure merged, then finish the policy decision in a separate patch.
I don't see the point in getting the infrastructure merged until this policy decision is made. And it seems to violate the "Don't comment code" coding standard of coresystems.
- If PCI IDs are not sufficient for board matching, the match can be further
- constrained by a string that has to be present in the DMI database for
- the baseboard or the system entry. The match type is case-sensitive
- substring match.
Maybe add some text which tells people to always include DMI IDs if they create new board matches. Not sure. It would certainly tighten board matching, and we don't know how bad the subsystem ID overlap between different boards is.
As written in the introduction text, this is something I would like to avoid. You have a point in "it's nice if it's always there", but this makes the "we have DMI because it's nice" and "we *need* to match the DMI info to be sure we have the right board" indistinguishable. I understand your point of tighter matches being better, but I don't think that's worth a hard dependency on dmidecode if we get along nicely without.
Some of the indentation in the block above uses spaces instead of tabs. Spaces are only OK if they are less than 8 and if they are preceded by tabs. The most common use case for spaces is indenting function arguments or expressions.
Yeah, I agree on that statement too. Fixed also at this place.
Please check the following patch.
<<commit message starts here>>
Matching via DMI: If a board is not uniquely identifiable by PCI device/subsystem IDs, a string can be specified to be looked for (case-sensitive, substring or anchored) for now in one of the following DMI items in addition to matching the PCI IDs: - System Manufacturer - System Product Name - System Version - Baseboard Manufacturer - Baseboard Product Name - Baseboard Version
Strings are anchored re-like (^ at the beginning, $ at the end), but there are no plans to support full regular expressions. The field to match is specified in a two-character prefix followed by a colon.
The match is only made if DMI info is available and the string matches. If no DMI info is available and the PCI IDs match, a warning is printed as the board can not be autodetected.
It's still open to discussion whether we add an DMI override switch to specify a string that will definitely match, and whether this switch is only used if no DMI is available or whether it overrides or augments DMI data.
DMI data is currently read using dmidecode. This tool is available for all major platforms except MacOS X. I heard that there also is a MacOS X version of dmidecode, but didn't investigate that.
Signed-Off-By: Michael Karcher flashrom@mkarcher.dialup.fu-berlin.de --- Changes in 3rd revision of the patch (all issues found by carldani): - Fixed spaces/tabs issues. - error checking on popen - DMI strings are dynamically allocated (size limit of 4096 shouldn't matter and avoids reallocations) - Specification of field to match is needed now - Added a DMI match for the P5A mainboard as example (untestd. Source for DMI info: http://enrico-scholz.de/hw/londo/dmidecode . - Minor coding style changes (spaces after "*" in pointer declaration, on pair of parentheses)
Index: flash.h =================================================================== --- flash.h (Revision 824) +++ flash.h (Arbeitskopie) @@ -251,6 +251,9 @@ uint16_t second_card_vendor; uint16_t second_card_device;
+ /* Pattern to match DMI entries */ + const char *dmi_pattern; + /* The vendor / part name from the coreboot table. */ const char *lb_vendor; const char *lb_part; @@ -330,6 +333,11 @@ extern char *lb_part, *lb_vendor; extern int partvendor_from_cbtable;
+/* dmi.c */ +extern int has_dmi_support; +void dmi_init(void); +int dmi_match(const char *pattern); + /* internal.c */ #if NEED_PCI == 1 struct superio { Index: Makefile =================================================================== --- Makefile (Revision 824) +++ Makefile (Arbeitskopie) @@ -101,7 +101,7 @@
ifeq ($(CONFIG_INTERNAL), yes) FEATURE_CFLAGS += -D'INTERNAL_SUPPORT=1' -PROGRAMMER_OBJS += chipset_enable.o board_enable.o cbtable.o it87spi.o ichspi.o sb600spi.o wbsio_spi.o +PROGRAMMER_OBJS += chipset_enable.o board_enable.o cbtable.o dmi.o it87spi.o ichspi.o sb600spi.o wbsio_spi.o NEED_PCI := yes endif
Index: dmi.c =================================================================== --- dmi.c (Revision 0) +++ dmi.c (Revision 0) @@ -0,0 +1,175 @@ +/* + * This file is part of the flashrom project. + * + * Copyright (C) 2009,2010 Michael Karcher + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include <string.h> +#include <stdio.h> +#include <stdlib.h> + +#include "flash.h" + +enum dmi_strings { + DMI_SYS_MANUFACTURER, + DMI_SYS_PRODUCT, + DMI_SYS_VERSION, + DMI_BB_MANUFACTURER, + DMI_BB_PRODUCT, + DMI_BB_VERSION, + DMI_ID_INVALID /* This must always be the last entry */ +}; + +/* The short_id for baseboard starts with "m" as in mainboard to leave + "b" available for BIOS */ +struct { + const char *dmidecode_name; + char short_id[3]; +} dmi_properties[DMI_ID_INVALID] = { + {"system-manufacturer", "sm"}, + {"system-product-name", "sp"}, + {"system-version", "sv"}, + {"baseboard-manufacturer", "mm"}, + {"baseboard-product-name", "mp"}, + {"baseboard-version", "mv"} +}; + +#define DMI_COMMAND_LEN_MAX 260 +const char *dmidecode_command = "dmidecode"; + +int has_dmi_support = 0; +char *dmistrings[DMI_ID_INVALID]; + +/* strings longer than 4096 in DMI are just insane */ +#define DMI_MAX_ANSWER_LEN 4096 + +void dmi_init(void) +{ + FILE *dmidecode_pipe; + int i; + char *answerbuf = malloc(DMI_MAX_ANSWER_LEN); + if(!answerbuf) + { + fprintf(stderr, "DMI: couldn't alloc answer buffer\n"); + return; + } + for (i = 0; i < DMI_ID_INVALID; i++) + { + char commandline[DMI_COMMAND_LEN_MAX+40]; + snprintf(commandline, sizeof(commandline), + "%s -s %s", dmidecode_command, + dmi_properties[i].dmidecode_name); + dmidecode_pipe = popen(commandline, "r"); + if (!dmidecode_pipe) + { + printf_debug("DMI pipe open error\n"); + goto out_free; + } + fgets(answerbuf, DMI_MAX_ANSWER_LEN, dmidecode_pipe); + if (ferror(dmidecode_pipe)) + { + printf_debug("DMI pipe read error\n"); + pclose(dmidecode_pipe); + goto out_free; + } + /* Toss all output above DMI_MAX_ANSWER_LEN away to prevent + deadlock on pclose. */ + while (!feof(dmidecode_pipe)) + getc(dmidecode_pipe); + if (pclose(dmidecode_pipe) != 0) + { + printf_debug("DMI pipe close error\n"); + goto out_free; + } + + /* chomp trailing newline */ + if (answerbuf[0] != 0 && + answerbuf[strlen(answerbuf) - 1] == '\n') + answerbuf[strlen(answerbuf) - 1] = 0; + printf_debug("DMI string %d: "%s"\n", i, answerbuf); + dmistrings[i] = strdup(answerbuf); + } + has_dmi_support = 1; +out_free: + free(answerbuf); +} + +/** + * Does an substring/prefix/postfix/whole-string match. + * + * The pattern is matched as-is. The only metacharacters supported are '^' + * at the beginning and '$' at the end. So you can look for "^prefix", + * "suffix$", "substring" or "^complete string$". + * + * @param value The string to check. + * @param pattern The pattern. + * @return Nonzero if pattern matches. + */ +static int dmi_compare(const char *value, const char *pattern) +{ + int anchored = 0; + int patternlen; + printf_debug("matching %s against %s\n", value, pattern); + /* The empty string is part of all strings */ + if (pattern[0] == 0) + return 1; + + if (pattern[0] == '^') { + anchored = 1; + pattern++; + } + + patternlen = strlen(pattern); + if (pattern[patternlen - 1] == '$') { + int valuelen = strlen(value); + patternlen--; + if(patternlen > valuelen) + return 0; + + /* full string match: require same length */ + if(anchored && (valuelen != patternlen)) + return 0; + + /* start character to make ends match */ + value += valuelen - patternlen; + anchored = 1; + } + + if (anchored) + return strncmp(value, pattern, patternlen) == 0; + else + return strstr(value, pattern) != NULL; +} + +int dmi_match(const char *pattern) +{ + int i; + if (!has_dmi_support) + return 0; + if (strlen(pattern) < 3 || pattern[2] != ':') + { + fprintf(stderr, "BUG: broken DMI pattern %s!\n", pattern); + return 0; + } + for (i = 0;i < DMI_ID_INVALID; i++) + { + if (strncmp(pattern, dmi_properties[i].short_id, 2) == 0) + return dmi_compare(dmistrings[i], pattern+3); + } + fprintf(stderr, "BUG: no DMI property %.2s\n", pattern); + return 0; +} Index: internal.c =================================================================== --- internal.c (Revision 824) +++ internal.c (Arbeitskopie) @@ -152,6 +152,7 @@ * mainboard specific flash enable sequence. */ coreboot_init(); + dmi_init();
/* Probe for the SuperI/O chip and fill global struct superio. */ probe_superio(); Index: board_enable.c =================================================================== --- board_enable.c (Revision 824) +++ board_enable.c (Arbeitskopie) @@ -1162,6 +1162,18 @@ * provide an as complete set of pci ids as possible; autodetection is the * preferred behaviour and we would like to make sure that matches are unique. * + * If PCI IDs are not sufficient for board matching, the match can be further + * constrained by a string that has to be present in the DMI database for + * the baseboard or the system entry. The DMI match uses a special pattern + * syntax: A field specifier (2 characters) followed by a colon and a pattern + * to match that field to. The field specifier can be "sm", "sp", "sv", "mm", + * "mp", "mv" for system manufacturer, system product name, system version, + * mainboard (aka base board) manufacturer, mainboard product name and + * mainboard version. The pattern after the colon is matched by case sensitve + * substring match, unless it is anchored to the beginning (with a ^ in front) + * or the end (with a $ at the end). Both anchors may be specified at the + * same time to match the full field. + * * The coreboot ids are used two fold. When running with a coreboot firmware, * the ids uniquely matches the coreboot board identification string. When a * legacy bios is installed and when autodetection is not possible, these ids @@ -1173,61 +1185,61 @@
/* Please keep this list alphabetically ordered by vendor/board name. */ struct board_pciid_enable board_pciid_enables[] = { - /* first pci-id set [4], second pci-id set [4], coreboot id [2], vendor name board name flash enable */ - {0x8086, 0x2926, 0x147b, 0x1084, 0x11ab, 0x4364, 0x147b, 0x1084, NULL, NULL, "Abit", "IP35", intel_ich_gpio16_raise}, - {0x105a, 0x0d30, 0x105a, 0x4d33, 0x8086, 0x1130, 0x8086, 0, NULL, NULL, "Acorp", "6A815EPD", board_acorp_6a815epd}, - {0x8086, 0x24D4, 0x1849, 0x24D0, 0x8086, 0x24D5, 0x1849, 0x9739, NULL, NULL, "ASRock", "P4i65GV", intel_ich_gpio23_raise}, - {0x1022, 0x746B, 0, 0, 0, 0, 0, 0, "AGAMI", "ARUMA", "agami", "Aruma", w83627hf_gpio24_raise_2e}, - {0x1106, 0x3177, 0x17F2, 0x3177, 0x1106, 0x3148, 0x17F2, 0x3148, NULL, NULL, "Albatron", "PM266A", w836xx_memw_enable_2e}, - {0x1106, 0x3205, 0x1106, 0x3205, 0x10EC, 0x8139, 0xA0A0, 0x0477, NULL, NULL, "AOpen", "vKM400Am-S", it8705_rom_write_enable}, - {0x1022, 0x2090, 0, 0, 0x1022, 0x2080, 0, 0, "artecgroup", "dbe61", "Artec Group", "DBE61", board_artecgroup_dbe6x}, - {0x1022, 0x2090, 0, 0, 0x1022, 0x2080, 0, 0, "artecgroup", "dbe62", "Artec Group", "DBE62", board_artecgroup_dbe6x}, - {0x1106, 0x3189, 0x1043, 0x807F, 0x1106, 0x3065, 0x1043, 0x80ED, NULL, NULL, "ASUS", "A7V600-X", board_asus_a7v600x}, - {0x1106, 0x3189, 0x1043, 0x807F, 0x1106, 0x3177, 0x1043, 0x808C, NULL, NULL, "ASUS", "A7V8X", board_asus_a7v8x}, - {0x1106, 0x3177, 0x1043, 0x80A1, 0x1106, 0x3205, 0x1043, 0x8118, NULL, NULL, "ASUS", "A7V8X-MX SE", w836xx_memw_enable_2e}, - {0x1106, 0x1336, 0x1043, 0x80ed, 0x1106, 0x3288, 0x1043, 0x8249, NULL, NULL, "ASUS", "M2V-MX", via_vt823x_gpio5_raise}, - {0x8086, 0x1a30, 0x1043, 0x8070, 0x8086, 0x244b, 0x1043, 0x8028, NULL, NULL, "ASUS", "P4B266", intel_ich_gpio22_raise}, - {0x8086, 0x1A30, 0x1043, 0x8025, 0x8086, 0x244B, 0x104D, 0x80F0, NULL, NULL, "ASUS", "P4B266-LM", intel_ich_gpio21_raise}, - {0x8086, 0x2570, 0x1043, 0x80F2, 0x105A, 0x3373, 0x1043, 0x80F5, NULL, NULL, "ASUS", "P4P800-E Deluxe", intel_ich_gpio21_raise}, - {0x10B9, 0x1541, 0, 0, 0x10B9, 0x1533, 0, 0, "asus", "p5a", "ASUS", "P5A", board_asus_p5a}, - {0x10DE, 0x0030, 0x1043, 0x818a, 0x8086, 0x100E, 0x1043, 0x80EE, NULL, NULL, "ASUS", "P5ND2-SLI Deluxe", nvidia_mcp_gpio10_raise}, - {0x1106, 0x3149, 0x1565, 0x3206, 0x1106, 0x3344, 0x1565, 0x1202, NULL, NULL, "Biostar", "P4M80-M4", it8705_rom_write_enable}, - {0x8086, 0x3590, 0x1028, 0x016c, 0x1000, 0x0030, 0x1028, 0x016c, NULL, NULL, "Dell", "PowerEdge 1850", intel_ich_gpio23_raise}, - {0x1039, 0x5513, 0x1019, 0x0A41, 0x1039, 0x0018, 0, 0, NULL, NULL, "Elitegroup", "K7S6A", elitegroup_k7vta3}, - {0x1106, 0x3038, 0x1019, 0x0996, 0x1106, 0x3177, 0x1019, 0x0996, NULL, NULL, "Elitegroup", "K7VTA3", elitegroup_k7vta3}, - {0x1106, 0x3177, 0x1106, 0x3177, 0x1106, 0x3059, 0x1695, 0x3005, NULL, NULL, "EPoX", "EP-8K5A2", w836xx_memw_enable_2e}, - {0x10EC, 0x8139, 0x1695, 0x9001, 0x11C1, 0x5811, 0x1695, 0x9015, NULL, NULL, "EPoX", "EP-8RDA3+", nvidia_mcp_gpio31_raise}, - {0x8086, 0x7110, 0, 0, 0x8086, 0x7190, 0, 0, "epox", "ep-bx3", "EPoX", "EP-BX3", board_epox_ep_bx3}, - {0x1039, 0x0761, 0, 0, 0x10EC, 0x8168, 0, 0, "gigabyte", "2761gxdk", "GIGABYTE", "GA-2761GXDK", it87xx_probe_spi_flash}, - {0x1106, 0x3227, 0x1458, 0x5001, 0x10ec, 0x8139, 0x1458, 0xe000, NULL, NULL, "GIGABYTE", "GA-7VT600", it8705_rom_write_enable}, - {0x10DE, 0x0050, 0x1458, 0x0C11, 0x10DE, 0x005e, 0x1458, 0x5000, NULL, NULL, "GIGABYTE", "GA-K8N-SLI", nvidia_mcp_gpio21_raise}, - {0x10DE, 0x0360, 0x1458, 0x0C11, 0x10DE, 0x0369, 0x1458, 0x5001, "gigabyte", "m57sli", "GIGABYTE", "GA-M57SLI-S4", it87xx_probe_spi_flash}, - {0x10de, 0x03e0, 0, 0, 0x10DE, 0x03D0, 0, 0, NULL, NULL, "GIGABYTE", "GA-M61P-S3", it87xx_probe_spi_flash}, - {0x1002, 0x4398, 0x1458, 0x5004, 0x1002, 0x4391, 0x1458, 0xb000, NULL, NULL, "GIGABYTE", "GA-MA78G-DS3H", it87xx_probe_spi_flash}, - {0x1002, 0x4398, 0x1458, 0x5004, 0x1002, 0x4391, 0x1458, 0xb002, NULL, NULL, "GIGABYTE", "GA-MA78GM-S2H", it87xx_probe_spi_flash}, - {0x1002, 0x438d, 0x1458, 0x5001, 0x1002, 0x5956, 0x1002, 0x5956, NULL, NULL, "GIGABYTE", "GA-MA790FX-DQ6", it87xx_probe_spi_flash}, - {0x1166, 0x0223, 0x103c, 0x320d, 0x102b, 0x0522, 0x103c, 0x31fa, "hp", "dl145_g3", "HP", "DL145 G3", board_hp_dl145_g3_enable}, - {0x1166, 0x0205, 0x1014, 0x0347, 0x1002, 0x515E, 0x1014, 0x0325, NULL, NULL, "IBM", "x3455", board_ibm_x3455}, - {0x1039, 0x5513, 0x8086, 0xd61f, 0x1039, 0x6330, 0x8086, 0xd61f, NULL, NULL, "Intel", "D201GLY", wbsio_check_for_spi}, - {0x1022, 0x7468, 0, 0, 0, 0, 0, 0, "iwill", "dk8_htx", "IWILL", "DK8-HTX", w83627hf_gpio24_raise_2e}, - {0x8086, 0x27A0, 0, 0, 0x8086, 0x27b8, 0, 0, "kontron", "986lcd-m", "Kontron", "986LCD-M", board_kontron_986lcd_m}, - {0x8086, 0x2411, 0x8086, 0x2411, 0x8086, 0x7125, 0x0e11, 0xb165, NULL, NULL, "Mitac", "6513WU", board_mitac_6513wu}, - {0x13f6, 0x0111, 0x1462, 0x5900, 0x1106, 0x3177, 0x1106, 0, NULL, NULL, "MSI", "MS-6590 (KT4 Ultra)",board_msi_kt4v}, - {0x1106, 0x3149, 0x1462, 0x7094, 0x10ec, 0x8167, 0x1462, 0x094c, NULL, NULL, "MSI", "MS-6702E (K8T Neo2-F)",w83627thf_gpio4_4_raise_2e}, - {0x1106, 0x0571, 0x1462, 0x7120, 0x1106, 0x3065, 0x1462, 0x7120, NULL, NULL, "MSI", "MS-6712 (KT4V)", board_msi_kt4v}, - {0x8086, 0x2658, 0x1462, 0x7046, 0x1106, 0x3044, 0x1462, 0x046d, NULL, NULL, "MSI", "MS-7046", intel_ich_gpio19_raise}, - {0x10DE, 0x005E, 0x1462, 0x7135, 0x10DE, 0x0050, 0x1462, 0x7135, "msi", "k8n-neo3", "MSI", "MS-7135 (K8N Neo3)", w83627thf_gpio4_4_raise_4e}, - {0x10DE, 0x005E, 0x1462, 0x7125, 0x10DE, 0x0052, 0x1462, 0x7125, NULL, NULL, "MSI", "K8N Neo4-F", nvidia_mcp_gpio2_raise}, - {0x1106, 0x3099, 0, 0, 0x1106, 0x3074, 0, 0, "shuttle", "ak31", "Shuttle", "AK31", w836xx_memw_enable_2e}, - {0x1106, 0x3104, 0x1297, 0xa238, 0x1106, 0x3059, 0x1297, 0xc063, NULL, NULL, "Shuttle", "AK38N", shuttle_ak38n}, - {0x10DE, 0x0050, 0x1297, 0x5036, 0x1412, 0x1724, 0x1297, 0x5036, NULL, NULL, "Shuttle", "FN25", board_shuttle_fn25}, - {0x1106, 0x3038, 0x0925, 0x1234, 0x1106, 0x3058, 0x15DD, 0x7609, NULL, NULL, "Soyo", "SY-7VCA", board_soyo_sy_7vca}, - {0x8086, 0x1076, 0x8086, 0x1176, 0x1106, 0x3059, 0x10f1, 0x2498, NULL, NULL, "Tyan", "S2498 (Tomcat K7M)", w836xx_memw_enable_2e}, - {0x1106, 0x3177, 0x1106, 0xAA01, 0x1106, 0x3123, 0x1106, 0xAA01, NULL, NULL, "VIA", "EPIA M/MII/...", via_vt823x_gpio15_raise}, - {0x1106, 0x0259, 0x1106, 0x3227, 0x1106, 0x3065, 0x1106, 0x3149, NULL, NULL, "VIA", "EPIA-N/NL", via_vt823x_gpio9_raise}, - {0x1106, 0x5337, 0x1458, 0xb003, 0x1106, 0x287e, 0x1106, 0x337e, NULL, NULL, "VIA", "PC3500G", it87xx_probe_spi_flash}, + /* first pci-id set [4], second pci-id set [4], dmi_identifier, coreboot id [2], vendor name board name flash enable */ + {0x8086, 0x2926, 0x147b, 0x1084, 0x11ab, 0x4364, 0x147b, 0x1084, NULL, NULL, NULL, "Abit", "IP35", intel_ich_gpio16_raise}, + {0x105a, 0x0d30, 0x105a, 0x4d33, 0x8086, 0x1130, 0x8086, 0, NULL, NULL, NULL, "Acorp", "6A815EPD", board_acorp_6a815epd}, + {0x8086, 0x24D4, 0x1849, 0x24D0, 0x8086, 0x24D5, 0x1849, 0x9739, NULL, NULL, NULL, "ASRock", "P4i65GV", intel_ich_gpio23_raise}, + {0x1022, 0x746B, 0, 0, 0, 0, 0, 0, NULL, "AGAMI", "ARUMA", "agami", "Aruma", w83627hf_gpio24_raise_2e}, + {0x1106, 0x3177, 0x17F2, 0x3177, 0x1106, 0x3148, 0x17F2, 0x3148, NULL, NULL, NULL, "Albatron", "PM266A", w836xx_memw_enable_2e}, + {0x1106, 0x3205, 0x1106, 0x3205, 0x10EC, 0x8139, 0xA0A0, 0x0477, NULL, NULL, NULL, "AOpen", "vKM400Am-S", it8705_rom_write_enable}, + {0x1022, 0x2090, 0, 0, 0x1022, 0x2080, 0, 0, NULL, "artecgroup", "dbe61", "Artec Group", "DBE61", board_artecgroup_dbe6x}, + {0x1022, 0x2090, 0, 0, 0x1022, 0x2080, 0, 0, NULL, "artecgroup", "dbe62", "Artec Group", "DBE62", board_artecgroup_dbe6x}, + {0x1106, 0x3189, 0x1043, 0x807F, 0x1106, 0x3065, 0x1043, 0x80ED, NULL, NULL, NULL, "ASUS", "A7V600-X", board_asus_a7v600x}, + {0x1106, 0x3189, 0x1043, 0x807F, 0x1106, 0x3177, 0x1043, 0x808C, NULL, NULL, NULL, "ASUS", "A7V8X", board_asus_a7v8x}, + {0x1106, 0x3177, 0x1043, 0x80A1, 0x1106, 0x3205, 0x1043, 0x8118, NULL, NULL, NULL, "ASUS", "A7V8X-MX SE", w836xx_memw_enable_2e}, + {0x1106, 0x1336, 0x1043, 0x80ed, 0x1106, 0x3288, 0x1043, 0x8249, NULL, NULL, NULL, "ASUS", "M2V-MX", via_vt823x_gpio5_raise}, + {0x8086, 0x1a30, 0x1043, 0x8070, 0x8086, 0x244b, 0x1043, 0x8028, NULL, NULL, NULL, "ASUS", "P4B266", intel_ich_gpio22_raise}, + {0x8086, 0x1A30, 0x1043, 0x8025, 0x8086, 0x244B, 0x104D, 0x80F0, NULL, NULL, NULL, "ASUS", "P4B266-LM", intel_ich_gpio21_raise}, + {0x8086, 0x2570, 0x1043, 0x80F2, 0x105A, 0x3373, 0x1043, 0x80F5, NULL, NULL, NULL, "ASUS", "P4P800-E Deluxe", intel_ich_gpio21_raise}, + {0x10B9, 0x1541, 0, 0, 0x10B9, 0x1533, 0, 0, "mp:^P5A$", "asus", "p5a", "ASUS", "P5A", board_asus_p5a}, + {0x10DE, 0x0030, 0x1043, 0x818a, 0x8086, 0x100E, 0x1043, 0x80EE, NULL, NULL, NULL, "ASUS", "P5ND2-SLI Deluxe", nvidia_mcp_gpio10_raise}, + {0x1106, 0x3149, 0x1565, 0x3206, 0x1106, 0x3344, 0x1565, 0x1202, NULL, NULL, NULL, "Biostar", "P4M80-M4", it8705_rom_write_enable}, + {0x8086, 0x3590, 0x1028, 0x016c, 0x1000, 0x0030, 0x1028, 0x016c, NULL, NULL, NULL, "Dell", "PowerEdge 1850", intel_ich_gpio23_raise}, + {0x1039, 0x5513, 0x1019, 0x0A41, 0x1039, 0x0018, 0, 0, NULL, NULL, NULL, "Elitegroup", "K7S6A", elitegroup_k7vta3}, + {0x1106, 0x3038, 0x1019, 0x0996, 0x1106, 0x3177, 0x1019, 0x0996, NULL, NULL, NULL, "Elitegroup", "K7VTA3", elitegroup_k7vta3}, + {0x1106, 0x3177, 0x1106, 0x3177, 0x1106, 0x3059, 0x1695, 0x3005, NULL, NULL, NULL, "EPoX", "EP-8K5A2", w836xx_memw_enable_2e}, + {0x10EC, 0x8139, 0x1695, 0x9001, 0x11C1, 0x5811, 0x1695, 0x9015, NULL, NULL, NULL, "EPoX", "EP-8RDA3+", nvidia_mcp_gpio31_raise}, + {0x8086, 0x7110, 0, 0, 0x8086, 0x7190, 0, 0, NULL, "epox", "ep-bx3", "EPoX", "EP-BX3", board_epox_ep_bx3}, + {0x1039, 0x0761, 0, 0, 0x10EC, 0x8168, 0, 0, NULL, "gigabyte", "2761gxdk", "GIGABYTE", "GA-2761GXDK", it87xx_probe_spi_flash}, + {0x1106, 0x3227, 0x1458, 0x5001, 0x10ec, 0x8139, 0x1458, 0xe000, NULL, NULL, NULL, "GIGABYTE", "GA-7VT600", it8705_rom_write_enable}, + {0x10DE, 0x0050, 0x1458, 0x0C11, 0x10DE, 0x005e, 0x1458, 0x5000, NULL, NULL, NULL, "GIGABYTE", "GA-K8N-SLI", nvidia_mcp_gpio21_raise}, + {0x10DE, 0x0360, 0x1458, 0x0C11, 0x10DE, 0x0369, 0x1458, 0x5001, NULL, "gigabyte", "m57sli", "GIGABYTE", "GA-M57SLI-S4", it87xx_probe_spi_flash}, + {0x10de, 0x03e0, 0, 0, 0x10DE, 0x03D0, 0, 0, NULL, NULL, NULL, "GIGABYTE", "GA-M61P-S3", it87xx_probe_spi_flash}, + {0x1002, 0x4398, 0x1458, 0x5004, 0x1002, 0x4391, 0x1458, 0xb000, NULL, NULL, NULL, "GIGABYTE", "GA-MA78G-DS3H", it87xx_probe_spi_flash}, + {0x1002, 0x4398, 0x1458, 0x5004, 0x1002, 0x4391, 0x1458, 0xb002, NULL, NULL, NULL, "GIGABYTE", "GA-MA78GM-S2H", it87xx_probe_spi_flash}, + {0x1002, 0x438d, 0x1458, 0x5001, 0x1002, 0x5956, 0x1002, 0x5956, NULL, NULL, NULL, "GIGABYTE", "GA-MA790FX-DQ6", it87xx_probe_spi_flash}, + {0x1166, 0x0223, 0x103c, 0x320d, 0x102b, 0x0522, 0x103c, 0x31fa, NULL, "hp", "dl145_g3", "HP", "DL145 G3", board_hp_dl145_g3_enable}, + {0x1166, 0x0205, 0x1014, 0x0347, 0x1002, 0x515E, 0x1014, 0x0325, NULL, NULL, NULL, "IBM", "x3455", board_ibm_x3455}, + {0x1039, 0x5513, 0x8086, 0xd61f, 0x1039, 0x6330, 0x8086, 0xd61f, NULL, NULL, NULL, "Intel", "D201GLY", wbsio_check_for_spi}, + {0x1022, 0x7468, 0, 0, 0, 0, 0, 0, NULL, "iwill", "dk8_htx", "IWILL", "DK8-HTX", w83627hf_gpio24_raise_2e}, + {0x8086, 0x27A0, 0, 0, 0x8086, 0x27b8, 0, 0, NULL, "kontron", "986lcd-m", "Kontron", "986LCD-M", board_kontron_986lcd_m}, + {0x8086, 0x2411, 0x8086, 0x2411, 0x8086, 0x7125, 0x0e11, 0xb165, NULL, NULL, NULL, "Mitac", "6513WU", board_mitac_6513wu}, + {0x13f6, 0x0111, 0x1462, 0x5900, 0x1106, 0x3177, 0x1106, 0, NULL, NULL, NULL, "MSI", "MS-6590 (KT4 Ultra)",board_msi_kt4v}, + {0x1106, 0x3149, 0x1462, 0x7094, 0x10ec, 0x8167, 0x1462, 0x094c, NULL, NULL, NULL, "MSI", "MS-6702E (K8T Neo2-F)",w83627thf_gpio4_4_raise_2e}, + {0x1106, 0x0571, 0x1462, 0x7120, 0x1106, 0x3065, 0x1462, 0x7120, NULL, NULL, NULL, "MSI", "MS-6712 (KT4V)", board_msi_kt4v}, + {0x8086, 0x2658, 0x1462, 0x7046, 0x1106, 0x3044, 0x1462, 0x046d, NULL, NULL, NULL, "MSI", "MS-7046", intel_ich_gpio19_raise}, + {0x10DE, 0x005E, 0x1462, 0x7135, 0x10DE, 0x0050, 0x1462, 0x7135, NULL, "msi", "k8n-neo3", "MSI", "MS-7135 (K8N Neo3)", w83627thf_gpio4_4_raise_4e}, + {0x10DE, 0x005E, 0x1462, 0x7125, 0x10DE, 0x0052, 0x1462, 0x7125, NULL, NULL, NULL, "MSI", "K8N Neo4-F", nvidia_mcp_gpio2_raise}, + {0x1106, 0x3099, 0, 0, 0x1106, 0x3074, 0, 0, NULL, "shuttle", "ak31", "Shuttle", "AK31", w836xx_memw_enable_2e}, + {0x1106, 0x3104, 0x1297, 0xa238, 0x1106, 0x3059, 0x1297, 0xc063, NULL, NULL, NULL, "Shuttle", "AK38N", shuttle_ak38n}, + {0x10DE, 0x0050, 0x1297, 0x5036, 0x1412, 0x1724, 0x1297, 0x5036, NULL, NULL, NULL, "Shuttle", "FN25", board_shuttle_fn25}, + {0x1106, 0x3038, 0x0925, 0x1234, 0x1106, 0x3058, 0x15DD, 0x7609, NULL, NULL, NULL, "Soyo", "SY-7VCA", board_soyo_sy_7vca}, + {0x8086, 0x1076, 0x8086, 0x1176, 0x1106, 0x3059, 0x10f1, 0x2498, NULL, NULL, NULL, "Tyan", "S2498 (Tomcat K7M)", w836xx_memw_enable_2e}, + {0x1106, 0x3177, 0x1106, 0xAA01, 0x1106, 0x3123, 0x1106, 0xAA01, NULL, NULL, NULL, "VIA", "EPIA M/MII/...", via_vt823x_gpio15_raise}, + {0x1106, 0x0259, 0x1106, 0x3227, 0x1106, 0x3065, 0x1106, 0x3149, NULL, NULL, NULL, "VIA", "EPIA-N/NL", via_vt823x_gpio9_raise}, + {0x1106, 0x5337, 0x1458, 0xb003, 0x1106, 0x287e, 0x1106, 0x337e, NULL, NULL, NULL, "VIA", "PC3500G", it87xx_probe_spi_flash},
- { 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL, NULL}, /* end marker */ + { 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL, NULL, NULL}, /* end marker */ };
/** @@ -1314,6 +1326,18 @@ } }
+ if (board->dmi_pattern) { + if (!has_dmi_support) { + fprintf(stderr, "WARNING: Can't autodetect %s %s," + " DMI info unavailable.\n", + board->vendor_name, board->board_name); + continue; + } else { + if (!dmi_match(board->dmi_pattern)) + continue; + } + } + return board; }
On Mon, Jan 04, 2010 at 03:15:11PM +0100, Michael Karcher wrote:
Hello Carl-Daniel,
thanks for your review.
There seems to be a fundamental opinion difference what DMI is used for between you on the one side and me and Luc on the other side. You seem to want to use DMI for a good board identification (which is fair enough, I also started with that intention), whereas now we decided to use DMI only as an emergency helper in case the PCI IDs seem obviously not good enough, which should be the exception instead of the regular case.
Using DMI as emergency rescue has the advantage that it still works on most systems even if DMI is unavailable (boards having DMI info can not be matched by PCI IDs with the matching policy of this patch). Collecting DMI info on boards with good PCI subsystem IDs (like the MSI boards that have the internal 4-digit product number BCD-coded in the subsystem IDs) is not useful, in my opinion.
The usage of DMI as breaker explains the loose matching policy of "any DMI string matches", as the IDs that should go into the table should be unique enough even with this weak policy (I for example can't imagine any HP system, as the manufacturer has already been established by subsystem manufacturer IDs, where "^VL420$" is going to match in the wrong string). If you want to use DMI as general board identification mechanism, independent of the PCI IDs, one would of course have to check more than one field.
Some extra info on dmi that not everyone is acutely aware of:
This is where the dmi standard came from: http://www.dmtf.org/standards/dmi/
What you can read there is:
"Due to the rapid advancement of DMTF technologies, such as CIM, the DMTF defined an "End of Life" process for its Desktop Management Interface (DMI), which ended March 31, 2005"
End-of-life was not announced then, no, the end-of-life process _ended_ almost 5 years ago.
In that light, depending on DMI too much is not a good idea going forward, and then the proposed usage only as an extra option to match a board, next to pci main ids and subsystem ids seems like safe bet.
Will try to provide a more complete answer to this email later.
Luc Verhaegen.
Am Montag, den 04.01.2010, 15:35 +0100 schrieb Luc Verhaegen:
Some extra info on dmi that not everyone is acutely aware of:
This is where the dmi standard came from: http://www.dmtf.org/standards/dmi/
What you can read there is:
"Due to the rapid advancement of DMTF technologies, such as CIM, the DMTF defined an "End of Life" process for its Desktop Management Interface (DMI), which ended March 31, 2005"
You are right, DMI is outdated. However that doesn't really affect us. DMI is a complicated bloated remote API to access lots of system information. It has been replaced by even more bloated and high-level APIs as CIM.
A small subset of the info DMI could retrieve is the system information provided by the BIOS. This information was provided by the "Desktop Management BIOS specification". This specification was renamed to "System Management BIOS specification" when it got part of the DMTF to show that it is only a piece that is needed to get DMI to work.
What dmidecode does is just a dump of the SMBIOS database. The name dmidecode is for historic reason, smbiosdecode would be more accurate. This database is standardized in the SMBIOS specification, which is still alive, see http://www.dmtf.org/standards/smbios/
The latest revision is v2.6.1 from April 2009.
Regards, Michael Karcher
On Mon, Jan 04, 2010 at 06:10:51PM +0100, Michael Karcher wrote:
Am Montag, den 04.01.2010, 15:35 +0100 schrieb Luc Verhaegen:
Some extra info on dmi that not everyone is acutely aware of:
This is where the dmi standard came from: http://www.dmtf.org/standards/dmi/
What you can read there is:
"Due to the rapid advancement of DMTF technologies, such as CIM, the DMTF defined an "End of Life" process for its Desktop Management Interface (DMI), which ended March 31, 2005"
You are right, DMI is outdated. However that doesn't really affect us. DMI is a complicated bloated remote API to access lots of system information. It has been replaced by even more bloated and high-level APIs as CIM.
A small subset of the info DMI could retrieve is the system information provided by the BIOS. This information was provided by the "Desktop Management BIOS specification". This specification was renamed to "System Management BIOS specification" when it got part of the DMTF to show that it is only a piece that is needed to get DMI to work.
What dmidecode does is just a dump of the SMBIOS database. The name dmidecode is for historic reason, smbiosdecode would be more accurate. This database is standardized in the SMBIOS specification, which is still alive, see http://www.dmtf.org/standards/smbios/
The latest revision is v2.6.1 from April 2009.
Regards, Michael Karcher
I am not trying to keep dmi out of flashrom any more. Pciids solve 95% of our matches, we need to fill in the other 5%. Sadly, dmi, whatever its state, is our next best option.
I am just warning that dmi might not be as tight and enduring as many people think it is, and that therefor it should be used with some care.
Making a dmi string part of the matching algorithm seems necessary for some boards. But it should be pretty much last in precedence, and only as an expansion, a further possibility for board matching.
Also, i own Jetway mini-itx boards for which dmi will not change a thing (but luckily, they do not require a board enable).
Luc Verhaegen.
Am Donnerstag, den 07.01.2010, 10:30 +0100 schrieb Luc Verhaegen:
I am just warning that dmi might not be as tight and enduring as many people think it is, and that therefor it should be used with some care.
For the tightness, I completely agree: DMI/SMBIOS matching is only as good as the strings the vendor provide. That's the same problem we have with subsystem IDs. And of course if the vendor just ignores susbsystem IDs, it is quite probable that it also doesn't care about DMI/SMBIOS strings, see your Jetway example below.
For the endurance, I think there will be no problem: The SMBIOS table interface will be as enduring as non-EFI PC Biosses. There is no end-of-life announcement for SMBIOS, and Windows (still in Windows 7) uses SMBIOS to retrieve the system information presented WMI. BIOS vendors will provide SMBIOS stuff because the brand customers need this feature (they can't explain the business customers why their board name doesn't appear in the WMI database), which means that even no-name boards will get the SMBIOS table, but perhaps with useless entries, see the tightness issue above.
Also, i own Jetway mini-itx boards for which dmi will not change a thing (but luckily, they do not require a board enable).
Regards, Michael Karcher
On Thu, Jan 07, 2010 at 11:29:32AM +0100, Michael Karcher wrote:
Am Donnerstag, den 07.01.2010, 10:30 +0100 schrieb Luc Verhaegen:
I am just warning that dmi might not be as tight and enduring as many people think it is, and that therefor it should be used with some care.
For the tightness, I completely agree: DMI/SMBIOS matching is only as good as the strings the vendor provide. That's the same problem we have with subsystem IDs. And of course if the vendor just ignores susbsystem IDs, it is quite probable that it also doesn't care about DMI/SMBIOS strings, see your Jetway example below.
For the endurance, I think there will be no problem: The SMBIOS table interface will be as enduring as non-EFI PC Biosses. There is no end-of-life announcement for SMBIOS, and Windows (still in Windows 7) uses SMBIOS to retrieve the system information presented WMI. BIOS vendors will provide SMBIOS stuff because the brand customers need this feature (they can't explain the business customers why their board name doesn't appear in the WMI database), which means that even no-name boards will get the SMBIOS table, but perhaps with useless entries, see the tightness issue above.
Also, i own Jetway mini-itx boards for which dmi will not change a thing (but luckily, they do not require a board enable).
Regards, Michael Karcher
Would have to fire up the jetways and verify whether it really us that empty.
Luc Verhaegen.
On 04.01.2010 15:15, Michael Karcher wrote:
There seems to be a fundamental opinion difference what DMI is used for between you on the one side and me and Luc on the other side. You seem to want to use DMI for a good board identification (which is fair enough, I also started with that intention), whereas now we decided to use DMI only as an emergency helper in case the PCI IDs seem obviously not good enough, which should be the exception instead of the regular case.
I also collect superiotool output for known good boards. We don't need superiotool output for those boards, but I believe that output may be useful for data mining later. That "later" may actually be now if we look at the efforts to integrate generic superio probing into flashrom. My desire to get dmidecode data for all boards is motivated by the same reasons.
Using DMI as emergency rescue has the advantage that it still works on most systems even if DMI is unavailable (boards having DMI info can not be matched by PCI IDs with the matching policy of this patch). Collecting DMI info on boards with good PCI subsystem IDs (like the MSI boards that have the internal 4-digit product number BCD-coded in the subsystem IDs) is not useful, in my opinion.
IMHO PCI ID matching should be the primary match (we seem to agree here) and DMI should only be used as additional match to narrow down the results (we seem to agree on that point as well).
The usage of DMI as breaker explains the loose matching policy of "any DMI string matches", as the IDs that should go into the table should be unique enough even with this weak policy (I for example can't imagine any HP system, as the manufacturer has already been established by subsystem manufacturer IDs, where "^VL420$" is going to match in the wrong string). If you want to use DMI as general board identification mechanism, independent of the PCI IDs, one would of course have to check more than one field.
What about old boards without subsystem IDs? We have no idea about the manufacturer on those boards, so matching any DMI string might indeed give us multiple matches.
Just to make this very clear: I consider a pure DMI match (without PCI matching) to be a really bad idea because it gives us two orthogonal sets of IDs (PCI+DMI) without any way to correlate them.
Am Montag, den 04.01.2010, 04:53 +0100 schrieb Carl-Daniel Hailfinger: [...]
+int has_dmi_support = 0; +char dmistrings[DMI_ID_INVALID][80];
I'm not so happy about the static allocation here. Can't we use malloc() for the read buffer and strdup() for filling in dmistrings?
Of course we can. On the other hand, I don't really see the point in using dynamic allocation here. If it's about wasted memory, we should approach "struct flashchips" first. The fixed-size arrays in it waste a lot more.
True, they do waste a lot. I want to morph struct flashchip into a readonly structure and keep modifiable parts in an extra (or overlap) structure.
Nevertheless, I change it to dynamic allocation. Too bad the nice getline function that reads one line and allocates dynamically is GNU-only and not standard C or at least POSIX.
If you think functions like getline would be useful, copy them into flashrom.c where we already have min() and max() and others.
fgets(dmistrings[i], sizeof(dmistrings[i]), dmidecode_pipe);Hm. Check the result of fgets()?
Omitted by choice. The result of fgets is useless. It returns NULL if an error occurred (we would like to know) or if EOF occurred with no characters read (which is valid if the BIOS doesn't specify the string we are trying to read). So instead of checking the ambiguous return value of fgets, I check the ferror flag afterwards.
OK.
+int dmi_match(const char * identifier) +{
- int i;
- if (!has_dmi_support)
return 0;- for (i = 0;i < DMI_ID_INVALID; i++)
if (dmi_compare(dmistrings[i], identifier))I believe the "match any of the strings" policy is fundamentally wrong. If you and Luc think specifying multiple strings for the match is overkill/overdesign, please do at least allow specifying the DMI string this should be compared with.
Fair enough. I added it. Specifying one string to match is not increasing complexity much, although it will be a three-character hit on the board_enable table (if you specify like "bp:^A8V MX$" for baseboard product match for example). Luc, please NACK if you think the it is too much.
This also documents the used string in case someone later wants to modify the matching in any way. There's always the ability to specify "any:" or something like that in case the board vendors store the same identifier in different DMI strings based on BIOS revision.
Since this is a policy decision, we could have the inner loop here inside #ifdef FIXME_POLICY_DECISION or whatever (resulting in a non-match for all inputs) to get the infrstructure merged, then finish the policy decision in a separate patch.
I don't see the point in getting the infrastructure merged until this policy decision is made. And it seems to violate the "Don't comment code" coding standard of coresystems.
The "don't comment code" is more directed at stuff which was commented out or placed inside #if 0 without explanatory comment. But I see your point and concur.
- If PCI IDs are not sufficient for board matching, the match can be further
- constrained by a string that has to be present in the DMI database for
- the baseboard or the system entry. The match type is case-sensitive
- substring match.
Maybe add some text which tells people to always include DMI IDs if they create new board matches. Not sure. It would certainly tighten board matching, and we don't know how bad the subsystem ID overlap between different boards is.
As written in the introduction text, this is something I would like to avoid. You have a point in "it's nice if it's always there", but this makes the "we have DMI because it's nice" and "we *need* to match the DMI info to be sure we have the right board" indistinguishable. I understand your point of tighter matches being better, but I don't think that's worth a hard dependency on dmidecode if we get along nicely without.
OK, as long as we have the dmidecode info around (mailing list post) in case someone later finds another board with the same PCI IDs.
Some of the indentation in the block above uses spaces instead of tabs. Spaces are only OK if they are less than 8 and if they are preceded by tabs. The most common use case for spaces is indenting function arguments or expressions.
Yeah, I agree on that statement too. Fixed also at this place.
Thanks.
Please check the following patch.
<<commit message starts here>>
Matching via DMI: If a board is not uniquely identifiable by PCI device/subsystem IDs, a string can be specified to be looked for (case-sensitive, substring or anchored) for now in one of the following DMI items in addition to matching the PCI IDs:
- System Manufacturer
- System Product Name
- System Version
- Baseboard Manufacturer
- Baseboard Product Name
- Baseboard Version
Strings are anchored re-like (^ at the beginning, $ at the end), but there are no plans to support full regular expressions. The field to match is specified in a two-character prefix followed by a colon.
The match is only made if DMI info is available and the string matches. If no DMI info is available and the PCI IDs match, a warning is printed as the board can not be autodetected.
It's still open to discussion whether we add an DMI override switch to specify a string that will definitely match, and whether this switch is only used if no DMI is available or whether it overrides or augments DMI data.
DMI data is currently read using dmidecode. This tool is available for all major platforms except MacOS X. I heard that there also is a MacOS X version of dmidecode, but didn't investigate that.
Signed-Off-By: Michael Karcher flashrom@mkarcher.dialup.fu-berlin.de
Changes in 3rd revision of the patch (all issues found by carldani):
- Fixed spaces/tabs issues.
- error checking on popen
- DMI strings are dynamically allocated (size limit of 4096 shouldn't
matter and avoids reallocations)
- Specification of field to match is needed now
- Added a DMI match for the P5A mainboard as example (untestd. Source
for DMI info: http://enrico-scholz.de/hw/londo/dmidecode .
- Minor coding style changes (spaces after "*" in pointer declaration,
on pair of parentheses)
Index: flash.h
--- flash.h (Revision 824) +++ flash.h (Arbeitskopie) @@ -251,6 +251,9 @@ uint16_t second_card_vendor; uint16_t second_card_device;
- /* Pattern to match DMI entries */
- const char *dmi_pattern;
- /* The vendor / part name from the coreboot table. */ const char *lb_vendor; const char *lb_part;
@@ -330,6 +333,11 @@ extern char *lb_part, *lb_vendor; extern int partvendor_from_cbtable;
+/* dmi.c */ +extern int has_dmi_support; +void dmi_init(void); +int dmi_match(const char *pattern);
/* internal.c */ #if NEED_PCI == 1 struct superio { Index: Makefile =================================================================== --- Makefile (Revision 824) +++ Makefile (Arbeitskopie) @@ -101,7 +101,7 @@
ifeq ($(CONFIG_INTERNAL), yes) FEATURE_CFLAGS += -D'INTERNAL_SUPPORT=1' -PROGRAMMER_OBJS += chipset_enable.o board_enable.o cbtable.o it87spi.o ichspi.o sb600spi.o wbsio_spi.o +PROGRAMMER_OBJS += chipset_enable.o board_enable.o cbtable.o dmi.o it87spi.o ichspi.o sb600spi.o wbsio_spi.o NEED_PCI := yes endif
Index: dmi.c
--- dmi.c (Revision 0) +++ dmi.c (Revision 0) @@ -0,0 +1,175 @@ +/*
- This file is part of the flashrom project.
- Copyright (C) 2009,2010 Michael Karcher
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License as published by
- the Free Software Foundation; either version 2 of the License, or
- (at your option) any later version.
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
- You should have received a copy of the GNU General Public License
- along with this program; if not, write to the Free Software
- Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
- */
+#include <string.h> +#include <stdio.h> +#include <stdlib.h>
+#include "flash.h"
+enum dmi_strings {
- DMI_SYS_MANUFACTURER,
- DMI_SYS_PRODUCT,
- DMI_SYS_VERSION,
- DMI_BB_MANUFACTURER,
- DMI_BB_PRODUCT,
- DMI_BB_VERSION,
- DMI_ID_INVALID /* This must always be the last entry */
+};
+/* The short_id for baseboard starts with "m" as in mainboard to leave
- "b" available for BIOS */
Hm. The "m" naming is inconsistent with the "BB" names above. Not that important, you can commit as is.
+struct {
- const char *dmidecode_name;
- char short_id[3];
+} dmi_properties[DMI_ID_INVALID] = {
- {"system-manufacturer", "sm"},
- {"system-product-name", "sp"},
- {"system-version", "sv"},
- {"baseboard-manufacturer", "mm"},
- {"baseboard-product-name", "mp"},
- {"baseboard-version", "mv"}
+};
+#define DMI_COMMAND_LEN_MAX 260 +const char *dmidecode_command = "dmidecode";
+int has_dmi_support = 0; +char *dmistrings[DMI_ID_INVALID];
+/* strings longer than 4096 in DMI are just insane */ +#define DMI_MAX_ANSWER_LEN 4096
Hehe, yes indeed.
+void dmi_init(void) +{
- FILE *dmidecode_pipe;
- int i;
- char *answerbuf = malloc(DMI_MAX_ANSWER_LEN);
- if(!answerbuf)
- {
fprintf(stderr, "DMI: couldn't alloc answer buffer\n");return;- }
- for (i = 0; i < DMI_ID_INVALID; i++)
- {
char commandline[DMI_COMMAND_LEN_MAX+40];snprintf(commandline, sizeof(commandline),"%s -s %s", dmidecode_command,dmi_properties[i].dmidecode_name);dmidecode_pipe = popen(commandline, "r");if (!dmidecode_pipe){printf_debug("DMI pipe open error\n");goto out_free;}fgets(answerbuf, DMI_MAX_ANSWER_LEN, dmidecode_pipe);if (ferror(dmidecode_pipe)){printf_debug("DMI pipe read error\n");pclose(dmidecode_pipe);goto out_free;
Logic question: If there is any error reading one DMI string, we skip reading all subsequent strings AFAICS. Is that intentional?
}/* Toss all output above DMI_MAX_ANSWER_LEN away to preventdeadlock on pclose. */while (!feof(dmidecode_pipe))getc(dmidecode_pipe);if (pclose(dmidecode_pipe) != 0){printf_debug("DMI pipe close error\n");goto out_free;}/* chomp trailing newline */if (answerbuf[0] != 0 &&answerbuf[strlen(answerbuf) - 1] == '\n')answerbuf[strlen(answerbuf) - 1] = 0;
I noticed that the three expressions above could be simplified if we calculateed strlen(answerbuf) unconditionally. Micro-optimization though, can be postponed.
printf_debug("DMI string %d: \"%s\"\n", i, answerbuf);dmistrings[i] = strdup(answerbuf);- }
- has_dmi_support = 1;
+out_free:
- free(answerbuf);
+}
+/**
- Does an substring/prefix/postfix/whole-string match.
- The pattern is matched as-is. The only metacharacters supported are '^'
- at the beginning and '$' at the end. So you can look for "^prefix",
- "suffix$", "substring" or "^complete string$".
- @param value The string to check.
- @param pattern The pattern.
- @return Nonzero if pattern matches.
- */
+static int dmi_compare(const char *value, const char *pattern) +{
- int anchored = 0;
- int patternlen;
- printf_debug("matching %s against %s\n", value, pattern);
- /* The empty string is part of all strings */
- if (pattern[0] == 0)
return 1;- if (pattern[0] == '^') {
anchored = 1;pattern++;- }
- patternlen = strlen(pattern);
- if (pattern[patternlen - 1] == '$') {
int valuelen = strlen(value);patternlen--;if(patternlen > valuelen)return 0;/* full string match: require same length */if(anchored && (valuelen != patternlen))return 0;/* start character to make ends match */value += valuelen - patternlen;anchored = 1;- }
- if (anchored)
return strncmp(value, pattern, patternlen) == 0;- else
return strstr(value, pattern) != NULL;+}
+int dmi_match(const char *pattern) +{
- int i;
- if (!has_dmi_support)
return 0;- if (strlen(pattern) < 3 || pattern[2] != ':')
- {
fprintf(stderr, "BUG: broken DMI pattern %s!\n", pattern);return 0;- }
- for (i = 0;i < DMI_ID_INVALID; i++)
- {
if (strncmp(pattern, dmi_properties[i].short_id, 2) == 0)return dmi_compare(dmistrings[i], pattern+3);- }
- fprintf(stderr, "BUG: no DMI property %.2s\n", pattern);
- return 0;
+} Index: internal.c =================================================================== --- internal.c (Revision 824) +++ internal.c (Arbeitskopie) @@ -152,6 +152,7 @@ * mainboard specific flash enable sequence. */ coreboot_init();
dmi_init();
/* Probe for the SuperI/O chip and fill global struct superio. */ probe_superio();
Index: board_enable.c
--- board_enable.c (Revision 824) +++ board_enable.c (Arbeitskopie) @@ -1162,6 +1162,18 @@
- provide an as complete set of pci ids as possible; autodetection is the
- preferred behaviour and we would like to make sure that matches are unique.
- If PCI IDs are not sufficient for board matching, the match can be further
- constrained by a string that has to be present in the DMI database for
- the baseboard or the system entry. The DMI match uses a special pattern
- syntax: A field specifier (2 characters) followed by a colon and a pattern
- to match that field to. The field specifier can be "sm", "sp", "sv", "mm",
- "mp", "mv" for system manufacturer, system product name, system version,
- mainboard (aka base board) manufacturer, mainboard product name and
- mainboard version. The pattern after the colon is matched by case sensitve
- substring match, unless it is anchored to the beginning (with a ^ in front)
- or the end (with a $ at the end). Both anchors may be specified at the
- same time to match the full field.
- The coreboot ids are used two fold. When running with a coreboot firmware,
- the ids uniquely matches the coreboot board identification string. When a
- legacy bios is installed and when autodetection is not possible, these ids
@@ -1173,61 +1185,61 @@
/* Please keep this list alphabetically ordered by vendor/board name. */ struct board_pciid_enable board_pciid_enables[] = {
- /* first pci-id set [4], second pci-id set [4], coreboot id [2], vendor name board name flash enable */
- {0x8086, 0x2926, 0x147b, 0x1084, 0x11ab, 0x4364, 0x147b, 0x1084, NULL, NULL, "Abit", "IP35", intel_ich_gpio16_raise},
- {0x105a, 0x0d30, 0x105a, 0x4d33, 0x8086, 0x1130, 0x8086, 0, NULL, NULL, "Acorp", "6A815EPD", board_acorp_6a815epd},
- {0x8086, 0x24D4, 0x1849, 0x24D0, 0x8086, 0x24D5, 0x1849, 0x9739, NULL, NULL, "ASRock", "P4i65GV", intel_ich_gpio23_raise},
- {0x1022, 0x746B, 0, 0, 0, 0, 0, 0, "AGAMI", "ARUMA", "agami", "Aruma", w83627hf_gpio24_raise_2e},
- {0x1106, 0x3177, 0x17F2, 0x3177, 0x1106, 0x3148, 0x17F2, 0x3148, NULL, NULL, "Albatron", "PM266A", w836xx_memw_enable_2e},
- {0x1106, 0x3205, 0x1106, 0x3205, 0x10EC, 0x8139, 0xA0A0, 0x0477, NULL, NULL, "AOpen", "vKM400Am-S", it8705_rom_write_enable},
- {0x1022, 0x2090, 0, 0, 0x1022, 0x2080, 0, 0, "artecgroup", "dbe61", "Artec Group", "DBE61", board_artecgroup_dbe6x},
- {0x1022, 0x2090, 0, 0, 0x1022, 0x2080, 0, 0, "artecgroup", "dbe62", "Artec Group", "DBE62", board_artecgroup_dbe6x},
- {0x1106, 0x3189, 0x1043, 0x807F, 0x1106, 0x3065, 0x1043, 0x80ED, NULL, NULL, "ASUS", "A7V600-X", board_asus_a7v600x},
- {0x1106, 0x3189, 0x1043, 0x807F, 0x1106, 0x3177, 0x1043, 0x808C, NULL, NULL, "ASUS", "A7V8X", board_asus_a7v8x},
- {0x1106, 0x3177, 0x1043, 0x80A1, 0x1106, 0x3205, 0x1043, 0x8118, NULL, NULL, "ASUS", "A7V8X-MX SE", w836xx_memw_enable_2e},
- {0x1106, 0x1336, 0x1043, 0x80ed, 0x1106, 0x3288, 0x1043, 0x8249, NULL, NULL, "ASUS", "M2V-MX", via_vt823x_gpio5_raise},
- {0x8086, 0x1a30, 0x1043, 0x8070, 0x8086, 0x244b, 0x1043, 0x8028, NULL, NULL, "ASUS", "P4B266", intel_ich_gpio22_raise},
- {0x8086, 0x1A30, 0x1043, 0x8025, 0x8086, 0x244B, 0x104D, 0x80F0, NULL, NULL, "ASUS", "P4B266-LM", intel_ich_gpio21_raise},
- {0x8086, 0x2570, 0x1043, 0x80F2, 0x105A, 0x3373, 0x1043, 0x80F5, NULL, NULL, "ASUS", "P4P800-E Deluxe", intel_ich_gpio21_raise},
- {0x10B9, 0x1541, 0, 0, 0x10B9, 0x1533, 0, 0, "asus", "p5a", "ASUS", "P5A", board_asus_p5a},
- {0x10DE, 0x0030, 0x1043, 0x818a, 0x8086, 0x100E, 0x1043, 0x80EE, NULL, NULL, "ASUS", "P5ND2-SLI Deluxe", nvidia_mcp_gpio10_raise},
- {0x1106, 0x3149, 0x1565, 0x3206, 0x1106, 0x3344, 0x1565, 0x1202, NULL, NULL, "Biostar", "P4M80-M4", it8705_rom_write_enable},
- {0x8086, 0x3590, 0x1028, 0x016c, 0x1000, 0x0030, 0x1028, 0x016c, NULL, NULL, "Dell", "PowerEdge 1850", intel_ich_gpio23_raise},
- {0x1039, 0x5513, 0x1019, 0x0A41, 0x1039, 0x0018, 0, 0, NULL, NULL, "Elitegroup", "K7S6A", elitegroup_k7vta3},
- {0x1106, 0x3038, 0x1019, 0x0996, 0x1106, 0x3177, 0x1019, 0x0996, NULL, NULL, "Elitegroup", "K7VTA3", elitegroup_k7vta3},
- {0x1106, 0x3177, 0x1106, 0x3177, 0x1106, 0x3059, 0x1695, 0x3005, NULL, NULL, "EPoX", "EP-8K5A2", w836xx_memw_enable_2e},
- {0x10EC, 0x8139, 0x1695, 0x9001, 0x11C1, 0x5811, 0x1695, 0x9015, NULL, NULL, "EPoX", "EP-8RDA3+", nvidia_mcp_gpio31_raise},
- {0x8086, 0x7110, 0, 0, 0x8086, 0x7190, 0, 0, "epox", "ep-bx3", "EPoX", "EP-BX3", board_epox_ep_bx3},
- {0x1039, 0x0761, 0, 0, 0x10EC, 0x8168, 0, 0, "gigabyte", "2761gxdk", "GIGABYTE", "GA-2761GXDK", it87xx_probe_spi_flash},
- {0x1106, 0x3227, 0x1458, 0x5001, 0x10ec, 0x8139, 0x1458, 0xe000, NULL, NULL, "GIGABYTE", "GA-7VT600", it8705_rom_write_enable},
- {0x10DE, 0x0050, 0x1458, 0x0C11, 0x10DE, 0x005e, 0x1458, 0x5000, NULL, NULL, "GIGABYTE", "GA-K8N-SLI", nvidia_mcp_gpio21_raise},
- {0x10DE, 0x0360, 0x1458, 0x0C11, 0x10DE, 0x0369, 0x1458, 0x5001, "gigabyte", "m57sli", "GIGABYTE", "GA-M57SLI-S4", it87xx_probe_spi_flash},
- {0x10de, 0x03e0, 0, 0, 0x10DE, 0x03D0, 0, 0, NULL, NULL, "GIGABYTE", "GA-M61P-S3", it87xx_probe_spi_flash},
- {0x1002, 0x4398, 0x1458, 0x5004, 0x1002, 0x4391, 0x1458, 0xb000, NULL, NULL, "GIGABYTE", "GA-MA78G-DS3H", it87xx_probe_spi_flash},
- {0x1002, 0x4398, 0x1458, 0x5004, 0x1002, 0x4391, 0x1458, 0xb002, NULL, NULL, "GIGABYTE", "GA-MA78GM-S2H", it87xx_probe_spi_flash},
- {0x1002, 0x438d, 0x1458, 0x5001, 0x1002, 0x5956, 0x1002, 0x5956, NULL, NULL, "GIGABYTE", "GA-MA790FX-DQ6", it87xx_probe_spi_flash},
- {0x1166, 0x0223, 0x103c, 0x320d, 0x102b, 0x0522, 0x103c, 0x31fa, "hp", "dl145_g3", "HP", "DL145 G3", board_hp_dl145_g3_enable},
- {0x1166, 0x0205, 0x1014, 0x0347, 0x1002, 0x515E, 0x1014, 0x0325, NULL, NULL, "IBM", "x3455", board_ibm_x3455},
- {0x1039, 0x5513, 0x8086, 0xd61f, 0x1039, 0x6330, 0x8086, 0xd61f, NULL, NULL, "Intel", "D201GLY", wbsio_check_for_spi},
- {0x1022, 0x7468, 0, 0, 0, 0, 0, 0, "iwill", "dk8_htx", "IWILL", "DK8-HTX", w83627hf_gpio24_raise_2e},
- {0x8086, 0x27A0, 0, 0, 0x8086, 0x27b8, 0, 0, "kontron", "986lcd-m", "Kontron", "986LCD-M", board_kontron_986lcd_m},
- {0x8086, 0x2411, 0x8086, 0x2411, 0x8086, 0x7125, 0x0e11, 0xb165, NULL, NULL, "Mitac", "6513WU", board_mitac_6513wu},
- {0x13f6, 0x0111, 0x1462, 0x5900, 0x1106, 0x3177, 0x1106, 0, NULL, NULL, "MSI", "MS-6590 (KT4 Ultra)",board_msi_kt4v},
- {0x1106, 0x3149, 0x1462, 0x7094, 0x10ec, 0x8167, 0x1462, 0x094c, NULL, NULL, "MSI", "MS-6702E (K8T Neo2-F)",w83627thf_gpio4_4_raise_2e},
- {0x1106, 0x0571, 0x1462, 0x7120, 0x1106, 0x3065, 0x1462, 0x7120, NULL, NULL, "MSI", "MS-6712 (KT4V)", board_msi_kt4v},
- {0x8086, 0x2658, 0x1462, 0x7046, 0x1106, 0x3044, 0x1462, 0x046d, NULL, NULL, "MSI", "MS-7046", intel_ich_gpio19_raise},
- {0x10DE, 0x005E, 0x1462, 0x7135, 0x10DE, 0x0050, 0x1462, 0x7135, "msi", "k8n-neo3", "MSI", "MS-7135 (K8N Neo3)", w83627thf_gpio4_4_raise_4e},
- {0x10DE, 0x005E, 0x1462, 0x7125, 0x10DE, 0x0052, 0x1462, 0x7125, NULL, NULL, "MSI", "K8N Neo4-F", nvidia_mcp_gpio2_raise},
- {0x1106, 0x3099, 0, 0, 0x1106, 0x3074, 0, 0, "shuttle", "ak31", "Shuttle", "AK31", w836xx_memw_enable_2e},
- {0x1106, 0x3104, 0x1297, 0xa238, 0x1106, 0x3059, 0x1297, 0xc063, NULL, NULL, "Shuttle", "AK38N", shuttle_ak38n},
- {0x10DE, 0x0050, 0x1297, 0x5036, 0x1412, 0x1724, 0x1297, 0x5036, NULL, NULL, "Shuttle", "FN25", board_shuttle_fn25},
- {0x1106, 0x3038, 0x0925, 0x1234, 0x1106, 0x3058, 0x15DD, 0x7609, NULL, NULL, "Soyo", "SY-7VCA", board_soyo_sy_7vca},
- {0x8086, 0x1076, 0x8086, 0x1176, 0x1106, 0x3059, 0x10f1, 0x2498, NULL, NULL, "Tyan", "S2498 (Tomcat K7M)", w836xx_memw_enable_2e},
- {0x1106, 0x3177, 0x1106, 0xAA01, 0x1106, 0x3123, 0x1106, 0xAA01, NULL, NULL, "VIA", "EPIA M/MII/...", via_vt823x_gpio15_raise},
- {0x1106, 0x0259, 0x1106, 0x3227, 0x1106, 0x3065, 0x1106, 0x3149, NULL, NULL, "VIA", "EPIA-N/NL", via_vt823x_gpio9_raise},
- {0x1106, 0x5337, 0x1458, 0xb003, 0x1106, 0x287e, 0x1106, 0x337e, NULL, NULL, "VIA", "PC3500G", it87xx_probe_spi_flash},
- /* first pci-id set [4], second pci-id set [4], dmi_identifier, coreboot id [2], vendor name board name flash enable */
- {0x8086, 0x2926, 0x147b, 0x1084, 0x11ab, 0x4364, 0x147b, 0x1084, NULL, NULL, NULL, "Abit", "IP35", intel_ich_gpio16_raise},
- {0x105a, 0x0d30, 0x105a, 0x4d33, 0x8086, 0x1130, 0x8086, 0, NULL, NULL, NULL, "Acorp", "6A815EPD", board_acorp_6a815epd},
- {0x8086, 0x24D4, 0x1849, 0x24D0, 0x8086, 0x24D5, 0x1849, 0x9739, NULL, NULL, NULL, "ASRock", "P4i65GV", intel_ich_gpio23_raise},
- {0x1022, 0x746B, 0, 0, 0, 0, 0, 0, NULL, "AGAMI", "ARUMA", "agami", "Aruma", w83627hf_gpio24_raise_2e},
- {0x1106, 0x3177, 0x17F2, 0x3177, 0x1106, 0x3148, 0x17F2, 0x3148, NULL, NULL, NULL, "Albatron", "PM266A", w836xx_memw_enable_2e},
- {0x1106, 0x3205, 0x1106, 0x3205, 0x10EC, 0x8139, 0xA0A0, 0x0477, NULL, NULL, NULL, "AOpen", "vKM400Am-S", it8705_rom_write_enable},
- {0x1022, 0x2090, 0, 0, 0x1022, 0x2080, 0, 0, NULL, "artecgroup", "dbe61", "Artec Group", "DBE61", board_artecgroup_dbe6x},
- {0x1022, 0x2090, 0, 0, 0x1022, 0x2080, 0, 0, NULL, "artecgroup", "dbe62", "Artec Group", "DBE62", board_artecgroup_dbe6x},
- {0x1106, 0x3189, 0x1043, 0x807F, 0x1106, 0x3065, 0x1043, 0x80ED, NULL, NULL, NULL, "ASUS", "A7V600-X", board_asus_a7v600x},
- {0x1106, 0x3189, 0x1043, 0x807F, 0x1106, 0x3177, 0x1043, 0x808C, NULL, NULL, NULL, "ASUS", "A7V8X", board_asus_a7v8x},
- {0x1106, 0x3177, 0x1043, 0x80A1, 0x1106, 0x3205, 0x1043, 0x8118, NULL, NULL, NULL, "ASUS", "A7V8X-MX SE", w836xx_memw_enable_2e},
- {0x1106, 0x1336, 0x1043, 0x80ed, 0x1106, 0x3288, 0x1043, 0x8249, NULL, NULL, NULL, "ASUS", "M2V-MX", via_vt823x_gpio5_raise},
- {0x8086, 0x1a30, 0x1043, 0x8070, 0x8086, 0x244b, 0x1043, 0x8028, NULL, NULL, NULL, "ASUS", "P4B266", intel_ich_gpio22_raise},
- {0x8086, 0x1A30, 0x1043, 0x8025, 0x8086, 0x244B, 0x104D, 0x80F0, NULL, NULL, NULL, "ASUS", "P4B266-LM", intel_ich_gpio21_raise},
- {0x8086, 0x2570, 0x1043, 0x80F2, 0x105A, 0x3373, 0x1043, 0x80F5, NULL, NULL, NULL, "ASUS", "P4P800-E Deluxe", intel_ich_gpio21_raise},
- {0x10B9, 0x1541, 0, 0, 0x10B9, 0x1533, 0, 0, "mp:^P5A$", "asus", "p5a", "ASUS", "P5A", board_asus_p5a},
- {0x10DE, 0x0030, 0x1043, 0x818a, 0x8086, 0x100E, 0x1043, 0x80EE, NULL, NULL, NULL, "ASUS", "P5ND2-SLI Deluxe", nvidia_mcp_gpio10_raise},
- {0x1106, 0x3149, 0x1565, 0x3206, 0x1106, 0x3344, 0x1565, 0x1202, NULL, NULL, NULL, "Biostar", "P4M80-M4", it8705_rom_write_enable},
- {0x8086, 0x3590, 0x1028, 0x016c, 0x1000, 0x0030, 0x1028, 0x016c, NULL, NULL, NULL, "Dell", "PowerEdge 1850", intel_ich_gpio23_raise},
- {0x1039, 0x5513, 0x1019, 0x0A41, 0x1039, 0x0018, 0, 0, NULL, NULL, NULL, "Elitegroup", "K7S6A", elitegroup_k7vta3},
- {0x1106, 0x3038, 0x1019, 0x0996, 0x1106, 0x3177, 0x1019, 0x0996, NULL, NULL, NULL, "Elitegroup", "K7VTA3", elitegroup_k7vta3},
- {0x1106, 0x3177, 0x1106, 0x3177, 0x1106, 0x3059, 0x1695, 0x3005, NULL, NULL, NULL, "EPoX", "EP-8K5A2", w836xx_memw_enable_2e},
- {0x10EC, 0x8139, 0x1695, 0x9001, 0x11C1, 0x5811, 0x1695, 0x9015, NULL, NULL, NULL, "EPoX", "EP-8RDA3+", nvidia_mcp_gpio31_raise},
- {0x8086, 0x7110, 0, 0, 0x8086, 0x7190, 0, 0, NULL, "epox", "ep-bx3", "EPoX", "EP-BX3", board_epox_ep_bx3},
- {0x1039, 0x0761, 0, 0, 0x10EC, 0x8168, 0, 0, NULL, "gigabyte", "2761gxdk", "GIGABYTE", "GA-2761GXDK", it87xx_probe_spi_flash},
- {0x1106, 0x3227, 0x1458, 0x5001, 0x10ec, 0x8139, 0x1458, 0xe000, NULL, NULL, NULL, "GIGABYTE", "GA-7VT600", it8705_rom_write_enable},
- {0x10DE, 0x0050, 0x1458, 0x0C11, 0x10DE, 0x005e, 0x1458, 0x5000, NULL, NULL, NULL, "GIGABYTE", "GA-K8N-SLI", nvidia_mcp_gpio21_raise},
- {0x10DE, 0x0360, 0x1458, 0x0C11, 0x10DE, 0x0369, 0x1458, 0x5001, NULL, "gigabyte", "m57sli", "GIGABYTE", "GA-M57SLI-S4", it87xx_probe_spi_flash},
- {0x10de, 0x03e0, 0, 0, 0x10DE, 0x03D0, 0, 0, NULL, NULL, NULL, "GIGABYTE", "GA-M61P-S3", it87xx_probe_spi_flash},
- {0x1002, 0x4398, 0x1458, 0x5004, 0x1002, 0x4391, 0x1458, 0xb000, NULL, NULL, NULL, "GIGABYTE", "GA-MA78G-DS3H", it87xx_probe_spi_flash},
- {0x1002, 0x4398, 0x1458, 0x5004, 0x1002, 0x4391, 0x1458, 0xb002, NULL, NULL, NULL, "GIGABYTE", "GA-MA78GM-S2H", it87xx_probe_spi_flash},
- {0x1002, 0x438d, 0x1458, 0x5001, 0x1002, 0x5956, 0x1002, 0x5956, NULL, NULL, NULL, "GIGABYTE", "GA-MA790FX-DQ6", it87xx_probe_spi_flash},
- {0x1166, 0x0223, 0x103c, 0x320d, 0x102b, 0x0522, 0x103c, 0x31fa, NULL, "hp", "dl145_g3", "HP", "DL145 G3", board_hp_dl145_g3_enable},
- {0x1166, 0x0205, 0x1014, 0x0347, 0x1002, 0x515E, 0x1014, 0x0325, NULL, NULL, NULL, "IBM", "x3455", board_ibm_x3455},
- {0x1039, 0x5513, 0x8086, 0xd61f, 0x1039, 0x6330, 0x8086, 0xd61f, NULL, NULL, NULL, "Intel", "D201GLY", wbsio_check_for_spi},
- {0x1022, 0x7468, 0, 0, 0, 0, 0, 0, NULL, "iwill", "dk8_htx", "IWILL", "DK8-HTX", w83627hf_gpio24_raise_2e},
- {0x8086, 0x27A0, 0, 0, 0x8086, 0x27b8, 0, 0, NULL, "kontron", "986lcd-m", "Kontron", "986LCD-M", board_kontron_986lcd_m},
- {0x8086, 0x2411, 0x8086, 0x2411, 0x8086, 0x7125, 0x0e11, 0xb165, NULL, NULL, NULL, "Mitac", "6513WU", board_mitac_6513wu},
- {0x13f6, 0x0111, 0x1462, 0x5900, 0x1106, 0x3177, 0x1106, 0, NULL, NULL, NULL, "MSI", "MS-6590 (KT4 Ultra)",board_msi_kt4v},
- {0x1106, 0x3149, 0x1462, 0x7094, 0x10ec, 0x8167, 0x1462, 0x094c, NULL, NULL, NULL, "MSI", "MS-6702E (K8T Neo2-F)",w83627thf_gpio4_4_raise_2e},
- {0x1106, 0x0571, 0x1462, 0x7120, 0x1106, 0x3065, 0x1462, 0x7120, NULL, NULL, NULL, "MSI", "MS-6712 (KT4V)", board_msi_kt4v},
- {0x8086, 0x2658, 0x1462, 0x7046, 0x1106, 0x3044, 0x1462, 0x046d, NULL, NULL, NULL, "MSI", "MS-7046", intel_ich_gpio19_raise},
- {0x10DE, 0x005E, 0x1462, 0x7135, 0x10DE, 0x0050, 0x1462, 0x7135, NULL, "msi", "k8n-neo3", "MSI", "MS-7135 (K8N Neo3)", w83627thf_gpio4_4_raise_4e},
- {0x10DE, 0x005E, 0x1462, 0x7125, 0x10DE, 0x0052, 0x1462, 0x7125, NULL, NULL, NULL, "MSI", "K8N Neo4-F", nvidia_mcp_gpio2_raise},
- {0x1106, 0x3099, 0, 0, 0x1106, 0x3074, 0, 0, NULL, "shuttle", "ak31", "Shuttle", "AK31", w836xx_memw_enable_2e},
- {0x1106, 0x3104, 0x1297, 0xa238, 0x1106, 0x3059, 0x1297, 0xc063, NULL, NULL, NULL, "Shuttle", "AK38N", shuttle_ak38n},
- {0x10DE, 0x0050, 0x1297, 0x5036, 0x1412, 0x1724, 0x1297, 0x5036, NULL, NULL, NULL, "Shuttle", "FN25", board_shuttle_fn25},
- {0x1106, 0x3038, 0x0925, 0x1234, 0x1106, 0x3058, 0x15DD, 0x7609, NULL, NULL, NULL, "Soyo", "SY-7VCA", board_soyo_sy_7vca},
- {0x8086, 0x1076, 0x8086, 0x1176, 0x1106, 0x3059, 0x10f1, 0x2498, NULL, NULL, NULL, "Tyan", "S2498 (Tomcat K7M)", w836xx_memw_enable_2e},
- {0x1106, 0x3177, 0x1106, 0xAA01, 0x1106, 0x3123, 0x1106, 0xAA01, NULL, NULL, NULL, "VIA", "EPIA M/MII/...", via_vt823x_gpio15_raise},
- {0x1106, 0x0259, 0x1106, 0x3227, 0x1106, 0x3065, 0x1106, 0x3149, NULL, NULL, NULL, "VIA", "EPIA-N/NL", via_vt823x_gpio9_raise},
- {0x1106, 0x5337, 0x1458, 0xb003, 0x1106, 0x287e, 0x1106, 0x337e, NULL, NULL, NULL, "VIA", "PC3500G", it87xx_probe_spi_flash},
- { 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL, NULL}, /* end marker */
- { 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL, NULL, NULL}, /* end marker */
};
/** @@ -1314,6 +1326,18 @@ } }
if (board->dmi_pattern) {if (!has_dmi_support) {fprintf(stderr, "WARNING: Can't autodetect %s %s,"" DMI info unavailable.\n",board->vendor_name, board->board_name);continue;} else {if (!dmi_match(board->dmi_pattern))continue;}}- return board; }
Acked-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Regards, Carl-Daniel
Am Mittwoch, den 06.01.2010, 13:58 +0100 schrieb Carl-Daniel Hailfinger:
On 04.01.2010 15:15, Michael Karcher wrote:
You seem to want to use DMI for a good board identification (which is fair enough, I also started with that intention), whereas now we decided to use DMI only as an emergency helper in case the PCI IDs seem obviously not good enough, which should be the exception instead of the regular case.
I also collect superiotool output for known good boards. We don't need superiotool output for those boards, but I believe that output may be useful for data mining later. [...] My desire to get dmidecode data for all boards is motivated by the same reasons.
OK, point taken. No problem
IMHO PCI ID matching should be the primary match (we seem to agree here) and DMI should only be used as additional match to narrow down the results (we seem to agree on that point as well).
Yes, definitely.
Just to make this very clear: I consider a pure DMI match (without PCI matching) to be a really bad idea because it gives us two orthogonal sets of IDs (PCI+DMI) without any way to correlate them.
Right. My first idea was to provide a DMI ID -> coreboot ID mapping that could be kept out of the board_enable structure. In that case, a pure DMI match would be used to identify the board, but then the usual PCI-ID cross checking you also have for coreboot IDs applies. Luc didn't like this idea, because it would scatter the information for one mainboard into two tables. I think his point is valid, which made the patch as it is now. I don't think one approach is technically much superior to the other one.
If you think functions like getline would be useful, copy them into flashrom.c where we already have min() and max() and others.
OK, will keep that in mind. It's not really needed here, though. The fixed-size buffer seems good enough.
Fair enough. I added it. Specifying one string to match is not increasing complexity much, although it will be a three-character hit on the board_enable table (if you specify like "bp:^A8V MX$" for baseboard
Oops, I trapped into the trap I set myself. It should be "mp:^A8V MX$" of course. But as adding BIOS identifiers could be needed soon, I really think using "m" for the baseboard is the lesser evil.
product match for example). Luc, please NACK if you think the it is too much.
This also documents the used string in case someone later wants to modify the matching in any way. There's always the ability to specify "any:" or something like that in case the board vendors store the same identifier in different DMI strings based on BIOS revision.
I don't think we need the "any:" prefix (I think I would have written it as "??:") at all. The match-any policy was in just to shorten the table entry.
As written in the introduction text, this is something I would like to avoid. You have a point in "it's nice if it's always there", but this makes the "we have DMI because it's nice" and "we *need* to match the DMI info to be sure we have the right board" indistinguishable.
[...]
OK, as long as we have the dmidecode info around (mailing list post) in case someone later finds another board with the same PCI IDs.
So, the bot command for "summary" should add "dmidecode output, but clear out your serial numbers/UUIDs".
if (ferror(dmidecode_pipe)){printf_debug("DMI pipe read error\n");pclose(dmidecode_pipe);goto out_free;Logic question: If there is any error reading one DMI string, we skip reading all subsequent strings AFAICS. Is that intentional?
The logic is that dmidecode should not fail at all. If reading one string fails, the goto (or return it was before the malloc() sneaked in) jumps over the has_dmi_support = 1 line, so DMI will be completely disabled of reading one string fails. As DMI will be completely disabled, reading further strings is useless. We might want to change that policy if it turns out that some dmidecode version doesn't implement some identifier strings.
/* chomp trailing newline */if (answerbuf[0] != 0 &&answerbuf[strlen(answerbuf) - 1] == '\n')answerbuf[strlen(answerbuf) - 1] = 0;I noticed that the three expressions above could be simplified if we calculateed strlen(answerbuf) unconditionally. Micro-optimization though, can be postponed.
Yeah, style question. It will introduce a temporary variable that's not in yet, but avoid the double strlen calculation. I trusted the gcc optimizer to catch it, but didn't check whether it really does.
Acked-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Thanks for your Ack. Luc, do you see any problems with the patch? Should we consider removing the string-selection prefix? I think the horse has been beaten to death and the code as it is now should be a fair compromise, but as the board enable table currently is kind of "your property", I would like to hear your opinion on it.
Regards, Michael Karcher
On 06.01.2010 14:27, Michael Karcher wrote:
Am Mittwoch, den 06.01.2010, 13:58 +0100 schrieb Carl-Daniel Hailfinger:
Just to make this very clear: I consider a pure DMI match (without PCI matching) to be a really bad idea because it gives us two orthogonal sets of IDs (PCI+DMI) without any way to correlate them.
Right. My first idea was to provide a DMI ID -> coreboot ID mapping that could be kept out of the board_enable structure. In that case, a pure DMI match would be used to identify the board, but then the usual PCI-ID cross checking you also have for coreboot IDs applies. Luc didn't like this idea, because it would scatter the information for one mainboard into two tables. I think his point is valid, which made the patch as it is now. I don't think one approach is technically much superior to the other one.
I agree with Luc here.
If you think functions like getline would be useful, copy them into flashrom.c where we already have min() and max() and others.
OK, will keep that in mind. It's not really needed here, though. The fixed-size buffer seems good enough.
OK.
On 04.01.2010 15:15, Michael Karcher wrote:
Fair enough. I added it. Specifying one string to match is not increasing complexity much, although it will be a three-character hit on the board_enable table (if you specify like "bp:^A8V MX$" for baseboard
Oops, I trapped into the trap I set myself. It should be "mp:^A8V MX$" of course. But as adding BIOS identifiers could be needed soon, I really think using "m" for the baseboard is the lesser evil.
Yes.
product match for example). Luc, please NACK if you think the it is too much.
This also documents the used string in case someone later wants to modify the matching in any way. There's always the ability to specify "any:" or something like that in case the board vendors store the same identifier in different DMI strings based on BIOS revision.
I don't think we need the "any:" prefix (I think I would have written it as "??:") at all. The match-any policy was in just to shorten the table entry.
Good point on "??:". If we ever need it, we can still implement it.
As written in the introduction text, this is something I would like to avoid. You have a point in "it's nice if it's always there", but this makes the "we have DMI because it's nice" and "we *need* to match the DMI info to be sure we have the right board" indistinguishable.
[...]
OK, as long as we have the dmidecode info around (mailing list post) in case someone later finds another board with the same PCI IDs.
So, the bot command for "summary" should add "dmidecode output, but clear out your serial numbers/UUIDs".
Absolutely. We could in theory tell people to run dmidecode six times (with a different string each time), but full dmidecode is likely easier. To be honest, I think creating a small script which collects all data would be user-friendly. That script should be hosted in svn.
if (ferror(dmidecode_pipe)){printf_debug("DMI pipe read error\n");pclose(dmidecode_pipe);goto out_free;Logic question: If there is any error reading one DMI string, we skip reading all subsequent strings AFAICS. Is that intentional?
The logic is that dmidecode should not fail at all. If reading one string fails, the goto (or return it was before the malloc() sneaked in) jumps over the has_dmi_support = 1 line, so DMI will be completely disabled of reading one string fails.
Ah, didn't see that. Good choice designing the logic like that.
As DMI will be completely disabled, reading further strings is useless. We might want to change that policy if it turns out that some dmidecode version doesn't implement some identifier strings.
Yes, let's wait and see.
/* chomp trailing newline */if (answerbuf[0] != 0 &&answerbuf[strlen(answerbuf) - 1] == '\n')answerbuf[strlen(answerbuf) - 1] = 0;I noticed that the three expressions above could be simplified if we calculateed strlen(answerbuf) unconditionally. Micro-optimization though, can be postponed.
Yeah, style question. It will introduce a temporary variable that's not in yet, but avoid the double strlen calculation. I trusted the gcc optimizer to catch it, but didn't check whether it really does.
It probably depends on the compiler, the compiler version and the optimizer flags. In the end, it's just a few nanoseconds we save and thus we should pick the code which is easier to read. No strong preference on my side.
Regards, Carl-Daniel
On Wed, Jan 06, 2010 at 03:19:54PM +0100, Carl-Daniel Hailfinger wrote:
To be honest, I think creating a small script which collects all data would be user-friendly. That script should be hosted in svn.
Sounds like a way better way of spending ones time than trying to code up ircbot scripts :) Its impact for users will be higher than expanding the ircbot, and the quality of information we get will improve.
Luc Verhaegen.
On Wed, Jan 06, 2010 at 02:27:28PM +0100, Michael Karcher wrote:
Am Mittwoch, den 06.01.2010, 13:58 +0100 schrieb Carl-Daniel Hailfinger:
Just to make this very clear: I consider a pure DMI match (without PCI matching) to be a really bad idea because it gives us two orthogonal sets of IDs (PCI+DMI) without any way to correlate them.
Right. My first idea was to provide a DMI ID -> coreboot ID mapping that could be kept out of the board_enable structure. In that case, a pure DMI match would be used to identify the board, but then the usual PCI-ID cross checking you also have for coreboot IDs applies. Luc didn't like this idea, because it would scatter the information for one mainboard into two tables. I think his point is valid, which made the patch as it is now. I don't think one approach is technically much superior to the other one.
It also tries to decouple DMI matching from the pciids and opens the door for full dmi handling (dmi info which might be altered when bioses are updated) and no pciid matching. This would end up being a big mess that at one point later we cannot fix anymore.
Fair enough. I added it. Specifying one string to match is not increasing complexity much, although it will be a three-character hit on the board_enable table (if you specify like "bp:^A8V MX$" for baseboard
Oops, I trapped into the trap I set myself. It should be "mp:^A8V MX$" of course. But as adding BIOS identifiers could be needed soon, I really think using "m" for the baseboard is the lesser evil.
Hrm. If this positional information is really necessary (and i think it isn't), then at least use an enum and get the compiler to catch typos.
I don't think we need the "any:" prefix (I think I would have written it as "??:") at all. The match-any policy was in just to shorten the table entry.
This whole thing is why i was initially very weary of *^$ in these strings. People end up trying to go beyond when they see something like that, it's something psychological that i have seen at play more often in these cases.
OK, as long as we have the dmidecode info around (mailing list post) in case someone later finds another board with the same PCI IDs.
So, the bot command for "summary" should add "dmidecode output, but clear out your serial numbers/UUIDs".
And even if those things are in there, who cares :)
The logic is that dmidecode should not fail at all. If reading one string fails, the goto (or return it was before the malloc() sneaked in) jumps over the has_dmi_support = 1 line, so DMI will be completely disabled of reading one string fails. As DMI will be completely disabled, reading further strings is useless. We might want to change that policy if it turns out that some dmidecode version doesn't implement some identifier strings.
Sounds good. Give up on dmi when it seems to bugger up is good enough not to get weird things happening. And the last statement reads as "only fix the problem if/when it presents itself" which is what i like for something like this, where it is clear that the issue can be worked around easily and cleanly.
Luc, do you see any problems with the patch? Should we consider removing the string-selection prefix? I think the horse has been beaten to death and the code as it is now should be a fair compromise, but as the board enable table currently is kind of "your property", I would like to hear your opinion on it.
As said, i do not like the string selection. _IF_ it is deemed really necessary, and i do not think it is, at least not yet, then we should use an enum and an extra entry.
For now, i would like to see just the string matching, and the relevant dmidecodes being collected so that possible future string selection can be implemented with some googling.
Luc Verhaegen.
On Mon, Jan 04, 2010 at 03:15:11PM +0100, Michael Karcher wrote:
Hello Carl-Daniel,
thanks for your review.
There seems to be a fundamental opinion difference what DMI is used for between you on the one side and me and Luc on the other side. You seem to want to use DMI for a good board identification (which is fair enough, I also started with that intention), whereas now we decided to use DMI only as an emergency helper in case the PCI IDs seem obviously not good enough, which should be the exception instead of the regular case.
Using DMI as emergency rescue has the advantage that it still works on most systems even if DMI is unavailable (boards having DMI info can not be matched by PCI IDs with the matching policy of this patch). Collecting DMI info on boards with good PCI subsystem IDs (like the MSI boards that have the internal 4-digit product number BCD-coded in the subsystem IDs) is not useful, in my opinion.
The usage of DMI as breaker explains the loose matching policy of "any DMI string matches", as the IDs that should go into the table should be unique enough even with this weak policy (I for example can't imagine any HP system, as the manufacturer has already been established by subsystem manufacturer IDs, where "^VL420$" is going to match in the wrong string). If you want to use DMI as general board identification mechanism, independent of the PCI IDs, one would of course have to check more than one field.
Agreed.
I'm not so happy about the static allocation here. Can't we use malloc() for the read buffer and strdup() for filling in dmistrings?
Of course we can. On the other hand, I don't really see the point in using dynamic allocation here. If it's about wasted memory, we should approach "struct flashchips" first. The fixed-size arrays in it waste a lot more. Nevertheless, I change it to dynamic allocation. Too bad the nice getline function that reads one line and allocates dynamically is GNU-only and not standard C or at least POSIX.
This is a run-once tool, the results are not stored, and we do not go and run dmidecode several times. Everything filling up any memory should be ran once.
Fair enough. I added it. Specifying one string to match is not increasing complexity much, although it will be a three-character hit on the board_enable table (if you specify like "bp:^A8V MX$" for baseboard product match for example). Luc, please NACK if you think the it is too much.
That's way too contrived for my taste. If you really believe it is necessary, then you should provide an enum entry right in front of the string.
But then the board enable table is taking another big hit, and this i would like to avoid at all cost. We already have pci ids and pci subsystem ids, this is just an extra bit of info that will be the last in precedence for any match.
As written in the introduction text, this is something I would like to avoid. You have a point in "it's nice if it's always there", but this makes the "we have DMI because it's nice" and "we *need* to match the DMI info to be sure we have the right board" indistinguishable. I understand your point of tighter matches being better, but I don't think that's worth a hard dependency on dmidecode if we get along nicely without.
I agree, this is a lowlevel tool and the less we depend on externals the better. That being said, if we have any board that requires dmidecode to be able to make a succesfull match, we already depend fully on dmidecode.
Luc Verhaegen.
Am Donnerstag, den 07.01.2010, 10:25 +0100 schrieb Luc Verhaegen:
I'm not so happy about the static allocation here. Can't we use malloc() for the read buffer and strdup() for filling in dmistrings?
Of course we can. On the other hand, I don't really see the point in using dynamic allocation here. If it's about wasted memory, we should approach "struct flashchips" first. The fixed-size arrays in it waste a lot more. Nevertheless, I change it to dynamic allocation. Too bad the nice getline function that reads one line and allocates dynamically is GNU-only and not standard C or at least POSIX.
This is a run-once tool, the results are not stored, and we do not go and run dmidecode several times. Everything filling up any memory should be ran once.
Sorry, I don't really understand your statement in this context. We do not run dmidecode once, but once for each string that might be interesting, because the machine-readable "-s" interface of dmidecode can only return one string at once. That's six invocations of dmidecode per flashrom start. The strings are collected during flashrom initialization and kept. The context of what you were replying to was whether the 6 dmi strings are stored in a statically allocated buffer or in 6 buffers allocated by malloc.
Fair enough. I added it. Specifying one string to match is not increasing complexity much, although it will be a three-character hit on the board_enable table (if you specify like "bp:^A8V MX$" for baseboard product match for example). Luc, please NACK if you think the it is too much.
That's way too contrived for my taste.
That's what I expected, and that's why I didn't commit yet. I won't commit until you and Carl-Daniel agree on the necessity of providing which string to match, although I slightly prefer the explicit specification of the string to match.
If you really believe it is necessary, then you should provide an enum entry right in front of the string.
Yeah. This would make the data structure cleaner...
But then the board enable table is taking another big hit, and this i would like to avoid at all cost.
... but I avoided it for exactly this reason. Especially because it will add only one empty column on boards that don't need DMI matching.
We already have pci ids and pci subsystem ids, this is just an extra bit of info that will be the last in precedence for any match.
Looking at the Asus P5A example (I know that this board is severly outdated, which might make this point less strong) there are no subsystem IDs at all in it. So when we start looking for DMI info we don't have any indication yet that we are on an Asus board. If we really have good subsystem IDs, we don't need DMI.
Regards, Michael Karcher
On Thu, Jan 07, 2010 at 11:16:14AM +0100, Michael Karcher wrote:
Am Donnerstag, den 07.01.2010, 10:25 +0100 schrieb Luc Verhaegen:
I'm not so happy about the static allocation here. Can't we use malloc() for the read buffer and strdup() for filling in dmistrings?
Of course we can. On the other hand, I don't really see the point in using dynamic allocation here. If it's about wasted memory, we should approach "struct flashchips" first. The fixed-size arrays in it waste a lot more. Nevertheless, I change it to dynamic allocation. Too bad the nice getline function that reads one line and allocates dynamically is GNU-only and not standard C or at least POSIX.
This is a run-once tool, the results are not stored, and we do not go and run dmidecode several times. Everything filling up any memory should be ran once.
Sorry, I don't really understand your statement in this context. We do not run dmidecode once, but once for each string that might be interesting, because the machine-readable "-s" interface of dmidecode can only return one string at once. That's six invocations of dmidecode per flashrom start. The strings are collected during flashrom initialization and kept. The context of what you were replying to was whether the 6 dmi strings are stored in a statically allocated buffer or in 6 buffers allocated by malloc.
Ok, then each of those 6 strings are only stored once, and are always stored (if dmidecode is present), the never need to get used for other purposes, they never need to get removed and re-added with other contents. They are local to the dmi.c code. I do not see a reason to make them dynamically allocated.
That's way too contrived for my taste.
That's what I expected, and that's why I didn't commit yet. I won't commit until you and Carl-Daniel agree on the necessity of providing which string to match, although I slightly prefer the explicit specification of the string to match.
If you really believe it is necessary, then you should provide an enum entry right in front of the string.
Yeah. This would make the data structure cleaner...
But then the board enable table is taking another big hit, and this i would like to avoid at all cost.
... but I avoided it for exactly this reason. Especially because it will add only one empty column on boards that don't need DMI matching.
We already have pci ids and pci subsystem ids, this is just an extra bit of info that will be the last in precedence for any match.
Looking at the Asus P5A example (I know that this board is severly outdated, which might make this point less strong) there are no subsystem IDs at all in it. So when we start looking for DMI info we don't have any indication yet that we are on an Asus board. If we really have good subsystem IDs, we don't need DMI.
Yeah, maybe a few of the other boards which can only be matched with -m can also be improved this way.
Luc Verhaegen.
On 07.01.2010 12:22, Luc Verhaegen wrote:
Ok, then each of those 6 strings are only stored once, and are always stored (if dmidecode is present), the never need to get used for other purposes, they never need to get removed and re-added with other contents. They are local to the dmi.c code. I do not see a reason to make them dynamically allocated.
My main motivation here was libflashrom which is almost ready. The CLI is already factored out, so someone may convert flashrom into a shared lib, and I thought a lib shouldn't have too many static allocations. I won't fight over this, there are more important problems to solve than dynamic vs static allocation.
Yeah, maybe a few of the other boards which can only be matched with -m can also be improved this way.
Now that would be awesome.
Regards, Carl-Daniel
On Thu, Jan 07, 2010 at 03:47:59PM +0100, Carl-Daniel Hailfinger wrote:
On 07.01.2010 12:22, Luc Verhaegen wrote:
Ok, then each of those 6 strings are only stored once, and are always stored (if dmidecode is present), the never need to get used for other purposes, they never need to get removed and re-added with other contents. They are local to the dmi.c code. I do not see a reason to make them dynamically allocated.
My main motivation here was libflashrom which is almost ready. The CLI is already factored out, so someone may convert flashrom into a shared lib, and I thought a lib shouldn't have too many static allocations. I won't fight over this, there are more important problems to solve than dynamic vs static allocation.
Ah, ok, in that case this might indeed be important, but i guess it is then better to go and clean up the lot in one go, as it will require interfaces to be created or altered.
Luc Verhaegen.
On 07.01.2010 11:16, Michael Karcher wrote:
Am Donnerstag, den 07.01.2010, 10:25 +0100 schrieb Luc Verhaegen:
Fair enough. I added it. Specifying one string to match is not increasing complexity much, although it will be a three-character hit on the board_enable table (if you specify like "bp:^A8V MX$" for baseboard product match for example). Luc, please NACK if you think the it is too much.
That's way too contrived for my taste.
That's what I expected, and that's why I didn't commit yet. I won't commit until you and Carl-Daniel agree on the necessity of providing which string to match, although I slightly prefer the explicit specification of the string to match.
The explicit string match has the advantage of being extensible. More below.
We already have pci ids and pci subsystem ids, this is just an extra bit of info that will be the last in precedence for any match.
Looking at the Asus P5A example (I know that this board is severly outdated, which might make this point less strong) there are no subsystem IDs at all in it. So when we start looking for DMI info we don't have any indication yet that we are on an Asus board. If we really have good subsystem IDs, we don't need DMI.
Yes. And in case we ever want to match two strings at once, the "bp:^A8V MX$" can be extended easily to "bvbp:^ASUS$:^A8V MX$" without changing any structs at all. Especially in the case where there is no usable PCI subsystem ID (board vendor not present) this may come handy. I'd wait for such an extension until we really desperately need it, though.
Regards, Carl-Daniel
On Thu, Jan 07, 2010 at 03:37:04PM +0100, Carl-Daniel Hailfinger wrote:
On 07.01.2010 11:16, Michael Karcher wrote:
That's what I expected, and that's why I didn't commit yet. I won't commit until you and Carl-Daniel agree on the necessity of providing which string to match, although I slightly prefer the explicit specification of the string to match.
The explicit string match has the advantage of being extensible. More below.
Looking at the Asus P5A example (I know that this board is severly outdated, which might make this point less strong) there are no subsystem IDs at all in it. So when we start looking for DMI info we don't have any indication yet that we are on an Asus board. If we really have good subsystem IDs, we don't need DMI.
Yes. And in case we ever want to match two strings at once, the "bp:^A8V MX$" can be extended easily to "bvbp:^ASUS$:^A8V MX$" without changing any structs at all. Especially in the case where there is no usable PCI subsystem ID (board vendor not present) this may come handy. I'd wait for such an extension until we really desperately need it, though.
From some previous mail:
I don't think we need the "any:" prefix (I think I would have written it as "??:") at all. The match-any policy was in just to shorten the table entry.
This whole thing is why i was initially very weary of *^$ in these strings. People end up trying to go beyond when they see something like that, it's something psychological that i have seen at play more often in these cases.
Luc Verhaegen.
On 07.01.2010 15:47, Luc Verhaegen wrote:
On Thu, Jan 07, 2010 at 03:37:04PM +0100, Carl-Daniel Hailfinger wrote:
On 07.01.2010 11:16, Michael Karcher wrote:
That's what I expected, and that's why I didn't commit yet. I won't commit until you and Carl-Daniel agree on the necessity of providing which string to match, although I slightly prefer the explicit specification of the string to match.
The explicit string match has the advantage of being extensible. More below.
Looking at the Asus P5A example (I know that this board is severly outdated, which might make this point less strong) there are no subsystem IDs at all in it. So when we start looking for DMI info we don't have any indication yet that we are on an Asus board. If we really have good subsystem IDs, we don't need DMI.
Yes. And in case we ever want to match two strings at once, the "bp:^A8V MX$" can be extended easily to "bvbp:^ASUS$:^A8V MX$" without changing any structs at all. Especially in the case where there is no usable PCI subsystem ID (board vendor not present) this may come handy. I'd wait for such an extension until we really desperately need it, though.
From some previous mail:
I don't think we need the "any:" prefix (I think I would have written it as "??:") at all. The match-any policy was in just to shorten the table entry.
This whole thing is why i was initially very weary of *^$ in these strings. People end up trying to go beyond when they see something like that, it's something psychological that i have seen at play more often in these cases.
Ah, I understood "go beyond" as trying to use unsupported regexps, which will be caught during review. My extensibility point was about not having to mess with the whole table each time we need changes in the DMI match policy.
Regards, Carl-Daniel
On Thu, Jan 07, 2010 at 04:01:10PM +0100, Carl-Daniel Hailfinger wrote:
On 07.01.2010 15:47, Luc Verhaegen wrote:
On Thu, Jan 07, 2010 at 03:37:04PM +0100, Carl-Daniel Hailfinger wrote:
On 07.01.2010 11:16, Michael Karcher wrote:
That's what I expected, and that's why I didn't commit yet. I won't commit until you and Carl-Daniel agree on the necessity of providing which string to match, although I slightly prefer the explicit specification of the string to match.
The explicit string match has the advantage of being extensible. More below.
Looking at the Asus P5A example (I know that this board is severly outdated, which might make this point less strong) there are no subsystem IDs at all in it. So when we start looking for DMI info we don't have any indication yet that we are on an Asus board. If we really have good subsystem IDs, we don't need DMI.
Yes. And in case we ever want to match two strings at once, the "bp:^A8V MX$" can be extended easily to "bvbp:^ASUS$:^A8V MX$" without changing any structs at all. Especially in the case where there is no usable PCI subsystem ID (board vendor not present) this may come handy. I'd wait for such an extension until we really desperately need it, though.
From some previous mail:
I don't think we need the "any:" prefix (I think I would have written it as "??:") at all. The match-any policy was in just to shorten the table entry.
This whole thing is why i was initially very weary of *^$ in these strings. People end up trying to go beyond when they see something like that, it's something psychological that i have seen at play more often in these cases.
Ah, I understood "go beyond" as trying to use unsupported regexps, which will be caught during review. My extensibility point was about not having to mess with the whole table each time we need changes in the DMI match policy.
I do not expect DMI many match policy changes.
* We've been relatively happy with pci subsystem ids for close to three years. * Our board enable table recently exploded, with the massive increase in flashrom uptake. But expect 30-40% of the board enables to be dropped soon (it8705f, w83627thf and itspi will be moved out), which will equally reduce our need to add entries to this table in future. * Even then a very large proportion of the boards is and will be happy with just pci subsystem ids. * 10-20% will be happy with pciids or pci subsystem ids and a simple DMI match.
A few percent, which we still have to encounter in our board enable table, will never be happy (like my jetway boards), and will always need -m.
The case where we need to match multiple strings just to be able to identify a board also needs to happen still.
As an example...
Let us consider a board where subsystem ids are just copied from the main ids (which is an issue of the bios, as the pci device default might either be 0 or copied depending on implemention).
Let's look at some use cases:
1)
0x1234, 0x5678, 0x1234, 0x5678, 0x1234, 0x5679, 0x1234, 0x5679
If 0x1234 is a popular manufacturer, and the 5678/5679 northbridge southbridge configuration is a popular product, then chances are that a few bad manufacturers really are this bad. But you will not find more than 2-3 vendors doing things this badly, at least not for the same chips. Chances are then very very high that the model name is unique, or that we can at least match the vendor.
2)
If we have the following for board maker 0xABCD:
0x1234, 0x5678, 0xABCD, 0x5678, 0x1234, 0x5679, 0xABCD, 0x5679
Then the match becomes a lot more unique already, and we should be able to easily get away with just the DMI product id.
3)
Even for a vendor who creates tons of boards with the same subsystem ids (and we know one which requires itspi a lot :)):
0x1234, 0x5678, 0xABCD, 0xEF12, 0x1234, 0x5679, 0xABCD, 0xEF12
We can then try to use the DMI product to differentiate the boards further (when choosing different pciids doesn't work, often due to added raid or gigE, that is). Here the string wildcard matching will help us out a lot, while the string selection will not.
This pretty much maps all three situations, and a combination thereof seems possible.
Now go back and try to see what "bvbp:^DEVICE$:^PRODUCT$" will gain us on top.
My current answer to this: not much.
Let's just see how it works out with simple matching with wildcards.
We will be collecting the data, and when needed, when a valid case turns up, we can expand.
Let's say that 30% of boards really require a gpio line to be poked. Of which 10% need dmi to help match the board. How high would you guess the amount of boards in those that cannot be helped with a simple match (with wildcards), but _can_ be helped with a more intricate match.
My current guess: 75% will work with a simple match. 10% will work when multiple entries are created. 10% will never work. The remaining 5%, and this is a _very_ _very_ high estimate, i expect it to be closer to 1%, is where intricate matching will be needed.
5 % of 10% of 30% = .15%
So yes, it is statistically possible that we might need it, but the probability is becoming _very_ low already, and before we encounter it, we will have written a whole lot of board enables.
Luc Verhaegen.
On 07.01.2010 16:46, Luc Verhaegen wrote:
I do not expect DMI many match policy changes.
- We've been relatively happy with pci subsystem ids for close to three years.
Yes, but that isn't a match-any policy either. The subsystem ID in the table must come from the same device as the main ID.
- Our board enable table recently exploded, with the massive increase in flashrom uptake.
Yes, and I'm very happy about that.
But expect 30-40% of the board enables to be dropped soon (it8705f, w83627thf and itspi will be moved out), which will equally reduce our need to add entries to this table in future.
Right.
- Even then a very large proportion of the boards is and will be happy with just pci subsystem ids.
Right.
- 10-20% will be happy with pciids or pci subsystem ids and a simple DMI match.
A few percent, which we still have to encounter in our board enable table, will never be happy (like my jetway boards), and will always need -m.
The case where we need to match multiple strings just to be able to identify a board also needs to happen still.
OK, so let's archive all interesting DMI strings on the list so we can investigate any overlaps that might happen in the future.
I would still like the match to be specific, though.
Let us consider a board where subsystem ids are just copied from the main ids (which is an issue of the bios, as the pci device default might either be 0 or copied depending on implemention).
Let's look at some use cases:
0x1234, 0x5678, 0x1234, 0x5678, 0x1234, 0x5679, 0x1234, 0x5679
If 0x1234 is a popular manufacturer, and the 5678/5679 northbridge southbridge configuration is a popular product, then chances are that a few bad manufacturers really are this bad. But you will not find more than 2-3 vendors doing things this badly, at least not for the same chips. Chances are then very very high that the model name is unique, or that we can at least match the vendor.
Note that our current DMI policy allows only either a vendor match or a board match, not both of them.
If we have the following for board maker 0xABCD:
0x1234, 0x5678, 0xABCD, 0x5678, 0x1234, 0x5679, 0xABCD, 0x5679
Then the match becomes a lot more unique already, and we should be able to easily get away with just the DMI product id.
Fully agreed.
Even for a vendor who creates tons of boards with the same subsystem ids (and we know one which requires itspi a lot :)):
0x1234, 0x5678, 0xABCD, 0xEF12, 0x1234, 0x5679, 0xABCD, 0xEF12
We can then try to use the DMI product to differentiate the boards further (when choosing different pciids doesn't work, often due to added raid or gigE, that is). Here the string wildcard matching will help us out a lot, while the string selection will not.
This pretty much maps all three situations, and a combination thereof seems possible.
Now go back and try to see what "bvbp:^DEVICE$:^PRODUCT$" will gain us on top.
My current answer to this: not much.
Yes, the multi-match with "bvbp:foo:bar" is something I do not want supported in current code. It may be useful sometime in the future, but for now I want to avoid such matches (and we seem to agree here).
Let's just see how it works out with simple matching with wildcards.
We will be collecting the data, and when needed, when a valid case turns up, we can expand.
Do you remember the pain you felt when going through old board enables which had missing subsystem IDs because back then people didn't care/know? Sure, there's always the possibility to dig through the mailing list archives, but it is rather painful. By specifying the string to be matched, I hope to avoid such pains in the future.
To summarize: - Specifying more than one DMI string for one board is hopefully something we will never have to do. - Not specifying which DMI string to match will be painful in case we ever have to adapt our DMI infrastructure.
Regards, Carl-Daniel
On Thu, Jan 07, 2010 at 05:25:32PM +0100, Carl-Daniel Hailfinger wrote:
On 07.01.2010 16:46, Luc Verhaegen wrote:
I do not expect DMI many match policy changes.
- We've been relatively happy with pci subsystem ids for close to three years.
Yes, but that isn't a match-any policy either. The subsystem ID in the table must come from the same device as the main ID.
Yes, but compared to the DMI, pciids are cheap and easy.
- 10-20% will be happy with pciids or pci subsystem ids and a simple DMI match.
A few percent, which we still have to encounter in our board enable table, will never be happy (like my jetway boards), and will always need -m.
The case where we need to match multiple strings just to be able to identify a board also needs to happen still.
OK, so let's archive all interesting DMI strings on the list so we can investigate any overlaps that might happen in the future.
I would still like the match to be specific, though.
This is what we've established now. We now require dmidecode output on the ml, before coding up a board enable.
Let us consider a board where subsystem ids are just copied from the main ids (which is an issue of the bios, as the pci device default might either be 0 or copied depending on implemention).
Let's look at some use cases:
0x1234, 0x5678, 0x1234, 0x5678, 0x1234, 0x5679, 0x1234, 0x5679
If 0x1234 is a popular manufacturer, and the 5678/5679 northbridge southbridge configuration is a popular product, then chances are that a few bad manufacturers really are this bad. But you will not find more than 2-3 vendors doing things this badly, at least not for the same chips. Chances are then very very high that the model name is unique, or that we can at least match the vendor.
Note that our current DMI policy allows only either a vendor match or a board match, not both of them.
This was not the point here, but let's dig out another example from another angle.
Let's think up a device, and fill out the strings we try to match:
DMI_SYS_VENDOR = ABC DMI_SYS_PRODUCT = XYZ DMI_SYS_VERSION = 1234 DMI_BB_VENDOR = ABC DMI_BB_PRODUCT = XYZ 123 DMI_BB_VERSION = 1234
Let's say there are multiple XYZ versions, that also require different board enables. And we need to match "XYZ 123" instead of just "XYZ". And then we can just as well state "^XYZ 123$", as we can "bv:^XYZ 123$".
What could require this string specific stuff, this example:
Two devices:
DMI_SYS_VENDOR = ABC DMI_SYS_PRODUCT = XYZ 123 DMI_SYS_VERSION = 1234 DMI_BB_VENDOR = ABC DMI_BB_PRODUCT = XYZ 123 (!) DMI_BB_VERSION = 1234
DMI_SYS_VENDOR = ABC DMI_SYS_PRODUCT = XYZ 123 DMI_SYS_VERSION = 1234 DMI_BB_VENDOR = ABC DMI_BB_PRODUCT = XYZ 124 (!) DMI_BB_VERSION = 1234
How likely is this? How likely is this _while_ they need a different board enable. (and do keep in mind the figures i tried to guess at in the last mail).
We are trying to design a highly complicated scheme here, before any such an issue ever occured, and even then, this issue will be in even less than the .15% margin guessed before.
If we have the following for board maker 0xABCD:
0x1234, 0x5678, 0xABCD, 0x5678, 0x1234, 0x5679, 0xABCD, 0x5679
Then the match becomes a lot more unique already, and we should be able to easily get away with just the DMI product id.
Fully agreed.
Even for a vendor who creates tons of boards with the same subsystem ids (and we know one which requires itspi a lot :)):
0x1234, 0x5678, 0xABCD, 0xEF12, 0x1234, 0x5679, 0xABCD, 0xEF12
We can then try to use the DMI product to differentiate the boards further (when choosing different pciids doesn't work, often due to added raid or gigE, that is). Here the string wildcard matching will help us out a lot, while the string selection will not.
This pretty much maps all three situations, and a combination thereof seems possible.
Now go back and try to see what "bvbp:^DEVICE$:^PRODUCT$" will gain us on top.
My current answer to this: not much.
Yes, the multi-match with "bvbp:foo:bar" is something I do not want supported in current code. It may be useful sometime in the future, but for now I want to avoid such matches (and we seem to agree here).
Let's just see how it works out with simple matching with wildcards.
We will be collecting the data, and when needed, when a valid case turns up, we can expand.
Do you remember the pain you felt when going through old board enables which had missing subsystem IDs because back then people didn't care/know? Sure, there's always the possibility to dig through the mailing list archives, but it is rather painful. By specifying the string to be matched, I hope to avoid such pains in the future.
This pain was trying to find _any_ good lspci. When i could just google for "$vendor $board lspci -v" and got good matches straight away, i was a happy camper. Being able to google "site:flashrom.org $vendor $board" and immediately get the relevant info (or even better, just grep the relevant mbox), is really really nice and fast.
To summarize:
- Specifying more than one DMI string for one board is hopefully
something we will never have to do.
The chance that this improves our matching potential on any board, is minimal, as explained before. It only will bring us something in a single or some _very_ specific cases.
- Not specifying which DMI string to match will be painful in case we
ever have to adapt our DMI infrastructure.
No, i doubt this is true.
First, i severely doubt this will ever occur.
Then if it does, we can deal with it then (we sure as hell know how already, that part has been bashed to death). If it does happen, then we can easily state _ANY for the existing ones, as this will be the main case anyway. If we want this to be tighter than any, then we can easily grep through our mailboxes and find the real matches.
Pain factor: very limited. I believe that the pain factor is mostly here, in another (imho) useless discussion over attempting to overdesign something instead of dealing with the real issues.
Deal with the real issue today, and finish off matching the remaining 5-10% of boards we cannot match today but which we can match with general string searching. The specification of the string is going to be a real margin requirement, that will only help us in a infinitessial number of cases, at best.
As i know that the "fix the issue when it presents itself" reasoning will not work in this case, and i can try to come up with the logic behind things endlessly, or try to come up with figures, i fear that it will not help in any way.
Try to work with this: Do it the way i suggest, and then prove me wrong.
Prove to me that there are actually cases that require this, by real examples, by waiting for the relevant case (yes, singular, no typo) to turn up.
Then feel free to give me all sorts of crap (i'm good at dishing it out, and therefor I should be good at getting it too (when justified)). I know that you are not the kind to kick people up the rear for something they did or decided wrong. But here i instruct you to do so, as that's the deal i want to see here.
Prove me wrong in that this will occur first, and give me the crap that i am asking for, and then if you give me enough crap, then I'll find the time to prove that it is not that hard to fix up too :)
Luc Verhaegen.
Am Donnerstag, den 31.12.2009, 17:22 +0100 schrieb Carl-Daniel Hailfinger:
thanks for your patch. Overall, I like it, but some design decisions may need to be revisited.
That doesn't sound too bad for something as new and still in discussion as this DMI patch. Thanks for your review.
On 30.12.2009 15:02, Michael Karcher wrote:
Matching via DMI: If a board is not uniquely identifiable by PCI device/subsystem IDs, a string can be specified to be looked for (case-sensitive, substring) for now in one of the following items:
- System Manufacturer
- System Product Name
- System Version
- Baseboard Manufacturer
- Baseboard Product Name
- Baseboard Version
I believe the substring match is suboptimal.
You got a point there.
For example, if you want to differentiate between the Asus M2A-VM and the Asus M2A-VM HDMI, only the HDMI variant can be matched safely.
The idea was that some manufacturers (HP I think) seem to put board ID and some revision or different ID into the same string, so we can't match full strings only.
This may be a stupid question, but how do AWDFLASH etc. match their boards? Their mechanism is designed to work well under most circumstances, and the manufacturers check it for ambiguities.
They don't do like we do. The vendor tools don't need to match the board to find out how to program it, because all vendor tools call into the BIOS or some code snippet provided with the BIOS update to do the board enable stuff. The only thing they have to check is whether the new BIOS matches the board. They do that by simply comparing some magic strings.
Vendors found out that one flash program that supports all boards is a horrible, unmaintainable mess of special cases, and as vendors can rely on their own BIOS in the running system, they just put the board enable code for that board into the BIOS. That's something we can't do with flashrom, as flashrom should work with coreboot, and coreboot currently does not support BIOS support.
The match is only made if DMI info is available and the string matches. If no DMI info is available, a warning is printed as the board can not be autodetected.
I'd make that warning conditional on the absence of a coreboot table. Otherwise you'll get a warning on a lot of systems with coreboot.
You will only get that warning if the PCI IDs match and the DMI is missing. When you run on coreboot, you don't even get into board_match_pci_card_ids(), as board_match_coreboot_name() already has chosen the right board.
It's still open to discussion whether we add an DMI override switch to specify a string that will definitely match, and whether this switch is only used if no DMI is available or whether it overrides or augments DMI data.
I think DMI should be used as additional matching help if PCI IDs are already correct. Especially in case multiple vendors have the same Product Name string, the PCI ID matching is crucial.
This is already implemented. DMI is only matched if PCI matches.
DMI data is currently read using dmidecode. This tool is available for all major platforms except MacOS X. I heard that there also is a MacOS X version of dmidecode, but didn't investigate that.
We might want to specify a dmidecode path/binary as optional internal programmer parameter for installations which don't have a native dmidecode in their path. Maybe something like flashrom -p internal:dmidecode=c:\cygwin\bin\dmidecode.exe That's something I'd leave to a followup patch, though.
I agree on that. And that makes agaran's point that the fixed-size command line buffer is bad more important. PATH_MAX+40 should be enough, and then snprintf for extra protection.
[three coding style hints snipped] Thanks again.
Regards, Michael Karcher
On Fri, Jan 01, 2010 at 01:27:44AM +0100, Michael Karcher wrote:
On 30.12.2009 15:02, Michael Karcher wrote:
Matching via DMI: If a board is not uniquely identifiable by PCI device/subsystem IDs, a string can be specified to be looked for (case-sensitive, substring) for now in one of the following items:
- System Manufacturer
- System Product Name
- System Version
- Baseboard Manufacturer
- Baseboard Product Name
- Baseboard Version
I believe the substring match is suboptimal.
You got a point there.
I wouldn't use any word related to optimal in relationship to the board enable table.
I would like to see a single string match, because this maximalises our effectivity, while minimising the impact on our already large board enable table.
That being said, sticking the dmi substring selection in the matching string, is contrived and will only lead to problems. If this really is what is wanted, then please use an enum.
But for the time being, i believe that it is not needed. And it can be added easily later on if we start collecting dmi info.
For example, if you want to differentiate between the Asus M2A-VM and the Asus M2A-VM HDMI, only the HDMI variant can be matched safely.
The idea was that some manufacturers (HP I think) seem to put board ID and some revision or different ID into the same string, so we can't match full strings only.
The HDMI variant here is implemented through a pci-express riser, so no difference whatsoever for us flashrommers. Egbert Eich (fellow radeonhd developer) has had a board like this for a few years now.
They don't do like we do. The vendor tools don't need to match the board to find out how to program it, because all vendor tools call into the BIOS or some code snippet provided with the BIOS update to do the board enable stuff. The only thing they have to check is whether the new BIOS matches the board. They do that by simply comparing some magic strings.
Vendors found out that one flash program that supports all boards is a horrible, unmaintainable mess of special cases, and as vendors can rely on their own BIOS in the running system, they just put the board enable code for that board into the BIOS. That's something we can't do with flashrom, as flashrom should work with coreboot, and coreboot currently does not support BIOS support.
We're finding out that the amount of board specific work is actually rather limited. With superio infrastructure (which i never seem to get round to testing) we can take out many more board enables. We then only are left with gpio lines, which, while common, are not needed for the majority of boards out there. And the infrastructure for them is a lot better already too.
I'd make that warning conditional on the absence of a coreboot table. Otherwise you'll get a warning on a lot of systems with coreboot.
You will only get that warning if the PCI IDs match and the DMI is missing. When you run on coreboot, you don't even get into board_match_pci_card_ids(), as board_match_coreboot_name() already has chosen the right board.
Sounds all good.
The matching functions is also still on my todo list. Splitting the matching functions from two (named/coreboot, pciid) into three (coreboot, named, pciid), to tighten up the board enable table. I never seem to get round to doing these things any more. I should make time, as it is rather important.
Luc Verhaegen.
On 07.01.2010 11:13, Luc Verhaegen wrote:
On Fri, Jan 01, 2010 at 01:27:44AM +0100, Michael Karcher wrote:
I believe the substring match is suboptimal.
You got a point there.
I wouldn't use any word related to optimal in relationship to the board enable table.
Heh. I tried to be polite.
I would like to see a single string match, because this maximalises our effectivity, while minimising the impact on our already large board enable table.
That being said, sticking the dmi substring selection in the matching string, is contrived and will only lead to problems. If this really is what is wanted, then please use an enum.
The enum will clutter up the table a lot more than 3 chars at the beginning of the DMI string, so I'd rather avoid it.
But for the time being, i believe that it is not needed. And it can be added easily later on if we start collecting dmi info.
We really need a database of all that stuff (DMI, PCIID, SuperIO) and not only something googleable. If google indexes the database, yay for us, but the ability to do systematic data mining does make stuff a lot easier.
I'd make that warning conditional on the absence of a coreboot table. Otherwise you'll get a warning on a lot of systems with coreboot.
You will only get that warning if the PCI IDs match and the DMI is missing. When you run on coreboot, you don't even get into board_match_pci_card_ids(), as board_match_coreboot_name() already has chosen the right board.
Sounds all good.
The matching functions is also still on my todo list. Splitting the matching functions from two (named/coreboot, pciid) into three (coreboot, named, pciid), to tighten up the board enable table.
Sorry, -ENOPARSE.
Regards, Carl-Daniel
On Thu, Jan 07, 2010 at 03:29:27PM +0100, Carl-Daniel Hailfinger wrote:
On 07.01.2010 11:13, Luc Verhaegen wrote:
On Fri, Jan 01, 2010 at 01:27:44AM +0100, Michael Karcher wrote:
I believe the substring match is suboptimal.
You got a point there.
I wouldn't use any word related to optimal in relationship to the board enable table.
Heh. I tried to be polite.
I was trying to be slightly amusing. The board enable table is big, both in width and length :) Suboptimal infers that there is a possibility to make this optimal, which is hard to imagine with the board enable table.
The enum will clutter up the table a lot more than 3 chars at the beginning of the DMI string, so I'd rather avoid it.
The prefix is A Bad Thing. As shown before, a typo is easily made, and we cannot check for it. If you require this sort of thing, an enum is the only option. The clutter we will then have to take with it.
That being said, i am still far from convinced that this is necessary.
The matching functions is also still on my todo list. Splitting the matching functions from two (named/coreboot, pciid) into three (coreboot, named, pciid), to tighten up the board enable table.
Sorry, -ENOPARSE.
This is the board enable matching tightening up i have been talking about for half a year or so now.
Luc Verhaegen.
On 07.01.2010 15:43, Luc Verhaegen wrote:
On Thu, Jan 07, 2010 at 03:29:27PM +0100, Carl-Daniel Hailfinger wrote:
On 07.01.2010 11:13, Luc Verhaegen wrote:
I wouldn't use any word related to optimal in relationship to the board enable table.
Heh. I tried to be polite.
I was trying to be slightly amusing.
:-)
The enum will clutter up the table a lot more than 3 chars at the beginning of the DMI string, so I'd rather avoid it.
The prefix is A Bad Thing. As shown before, a typo is easily made, and we cannot check for it.
Have you seen the selfcheck() function which is called on startup? I have patches which extend it a bit to check for eraseblock definition correctness, and it could easily check for string correctness on startup as well.
The matching functions is also still on my todo list. Splitting the matching functions from two (named/coreboot, pciid) into three (coreboot, named, pciid), to tighten up the board enable table.
Sorry, -ENOPARSE.
This is the board enable matching tightening up i have been talking about for half a year or so now.
Ah OK.
Regards, Carl-Daniel
On Thu, Jan 07, 2010 at 03:50:19PM +0100, Carl-Daniel Hailfinger wrote:
On 07.01.2010 15:43, Luc Verhaegen wrote:
The prefix is A Bad Thing. As shown before, a typo is easily made, and we cannot check for it.
Have you seen the selfcheck() function which is called on startup? I have patches which extend it a bit to check for eraseblock definition correctness, and it could easily check for string correctness on startup as well.
Ah, no i haven't, but on the other hand, enums are meant to enable typechecking, it's provided, it's compile time, etc.
Luc Verhaegen.
Michael, Luc,
please commit the version you two agree upon. I'm fine with all solutions as long as we get this committed.
Regards, Carl-Daniel
Am Dienstag, den 19.01.2010, 02:36 +0100 schrieb Carl-Daniel Hailfinger:
please commit the version you two agree upon. I'm fine with all solutions as long as we get this committed.
I will resubmit the current patch (revision 3, http://patchwork.coreboot.org/patch/730/) with the prefix stuff removed. This should be what we agreed on. This will be revision 2 + improved error checking + dynamic allocation + fixed code layout + the example match of the Asus P5A board.
Regards, Michael Karcher
If a board is not uniquely identifiable by PCI device/subsystem IDs, a string can be specified to be looked for (case-sensitive, substring or anchored) for now in one of the following DMI items in addition to matching the PCI IDs: - System Manufacturer - System Product Name - System Version - Baseboard Manufacturer - Baseboard Product Name - Baseboard Version
Strings are anchored re-like (^ at the beginning, $ at the end), but there are no plans to support full regular expressions and matched to any of the mentioned fields.
The match is only made if DMI info is available and the string matches. If no DMI info is available and the PCI IDs match, a warning is printed as the board can not be autodetected.
It's still open to discussion whether we add an DMI override switch to specify a string that will definitely match, and whether this switch is only used if no DMI is available or whether it overrides or augments DMI data.
DMI data is currently read using dmidecode. This tool is available for all major platforms except MacOS X. I heard that there also is a MacOS X version of dmidecode, but didn't investigate that.
Signed-off-by: Michael Karcher flashrom@mkarcher.dialup.fu-berlin.de --- Makefile | 2 +- board_enable.c | 131 +++++++++++++++++++++++++------------------- dmi.c | 168 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ flash.h | 8 +++ internal.c | 1 + 5 files changed, 253 insertions(+), 57 deletions(-) create mode 100644 dmi.c
diff --git a/Makefile b/Makefile index 501f65a..54f7c3d 100644 --- a/Makefile +++ b/Makefile @@ -101,7 +101,7 @@ CONFIG_PRINT_WIKI ?= no
ifeq ($(CONFIG_INTERNAL), yes) FEATURE_CFLAGS += -D'INTERNAL_SUPPORT=1' -PROGRAMMER_OBJS += chipset_enable.o board_enable.o cbtable.o it87spi.o ichspi.o sb600spi.o wbsio_spi.o +PROGRAMMER_OBJS += chipset_enable.o board_enable.o cbtable.o dmi.o it87spi.o ichspi.o sb600spi.o wbsio_spi.o NEED_PCI := yes endif
diff --git a/board_enable.c b/board_enable.c index 2446ddd..2daf8d1 100644 --- a/board_enable.c +++ b/board_enable.c @@ -1193,6 +1193,13 @@ static int board_asus_a7v600x(const char *name) * provide an as complete set of pci ids as possible; autodetection is the * preferred behaviour and we would like to make sure that matches are unique. * + * If PCI IDs are not sufficient for board matching, the match can be further + * constrained by a string that has to be present in the DMI database for + * the baseboard or the system entry. The pattern is matched by case sensitve + * substring match, unless it is anchored to the beginning (with a ^ in front) + * or the end (with a $ at the end). Both anchors may be specified at the + * same time to match the full field. + * * The coreboot ids are used two fold. When running with a coreboot firmware, * the ids uniquely matches the coreboot board identification string. When a * legacy bios is installed and when autodetection is not possible, these ids @@ -1204,62 +1211,62 @@ static int board_asus_a7v600x(const char *name)
/* Please keep this list alphabetically ordered by vendor/board name. */ struct board_pciid_enable board_pciid_enables[] = { - /* first pci-id set [4], second pci-id set [4], coreboot id [2], vendor name board name flash enable */ - {0x8086, 0x2926, 0x147b, 0x1084, 0x11ab, 0x4364, 0x147b, 0x1084, NULL, NULL, "Abit", "IP35", intel_ich_gpio16_raise}, - {0x105a, 0x0d30, 0x105a, 0x4d33, 0x8086, 0x1130, 0x8086, 0, NULL, NULL, "Acorp", "6A815EPD", board_acorp_6a815epd}, - {0x8086, 0x24D4, 0x1849, 0x24D0, 0x8086, 0x24D5, 0x1849, 0x9739, NULL, NULL, "ASRock", "P4i65GV", intel_ich_gpio23_raise}, - {0x1022, 0x746B, 0, 0, 0, 0, 0, 0, "AGAMI", "ARUMA", "agami", "Aruma", w83627hf_gpio24_raise_2e}, - {0x1106, 0x3177, 0x17F2, 0x3177, 0x1106, 0x3148, 0x17F2, 0x3148, NULL, NULL, "Albatron", "PM266A", w836xx_memw_enable_2e}, - {0x1106, 0x3205, 0x1106, 0x3205, 0x10EC, 0x8139, 0xA0A0, 0x0477, NULL, NULL, "AOpen", "vKM400Am-S", it8705_rom_write_enable}, - {0x1022, 0x2090, 0, 0, 0x1022, 0x2080, 0, 0, "artecgroup", "dbe61", "Artec Group", "DBE61", board_artecgroup_dbe6x}, - {0x1022, 0x2090, 0, 0, 0x1022, 0x2080, 0, 0, "artecgroup", "dbe62", "Artec Group", "DBE62", board_artecgroup_dbe6x}, - {0x1106, 0x3189, 0x1043, 0x807F, 0x1106, 0x3065, 0x1043, 0x80ED, NULL, NULL, "ASUS", "A7V600-X", board_asus_a7v600x}, - {0x1106, 0x3189, 0x1043, 0x807F, 0x1106, 0x3177, 0x1043, 0x808C, NULL, NULL, "ASUS", "A7V8X", board_asus_a7v8x}, - {0x1106, 0x3177, 0x1043, 0x80A1, 0x1106, 0x3205, 0x1043, 0x8118, NULL, NULL, "ASUS", "A7V8X-MX SE", w836xx_memw_enable_2e}, - {0x1106, 0x1336, 0x1043, 0x80ed, 0x1106, 0x3288, 0x1043, 0x8249, NULL, NULL, "ASUS", "M2V-MX", via_vt823x_gpio5_raise}, - {0x8086, 0x1a30, 0x1043, 0x8070, 0x8086, 0x244b, 0x1043, 0x8028, NULL, NULL, "ASUS", "P4B266", intel_ich_gpio22_raise}, - {0x8086, 0x1A30, 0x1043, 0x8025, 0x8086, 0x244B, 0x104D, 0x80F0, NULL, NULL, "ASUS", "P4B266-LM", intel_ich_gpio21_raise}, - {0x8086, 0x2570, 0x1043, 0x80F2, 0x105A, 0x3373, 0x1043, 0x80F5, NULL, NULL, "ASUS", "P4P800-E Deluxe", intel_ich_gpio21_raise}, - {0x10B9, 0x1541, 0, 0, 0x10B9, 0x1533, 0, 0, "asus", "p5a", "ASUS", "P5A", board_asus_p5a}, - {0x10DE, 0x0030, 0x1043, 0x818a, 0x8086, 0x100E, 0x1043, 0x80EE, NULL, NULL, "ASUS", "P5ND2-SLI Deluxe", nvidia_mcp_gpio10_raise}, - {0x1106, 0x3149, 0x1565, 0x3206, 0x1106, 0x3344, 0x1565, 0x1202, NULL, NULL, "Biostar", "P4M80-M4", it8705_rom_write_enable}, - {0x8086, 0x3590, 0x1028, 0x016c, 0x1000, 0x0030, 0x1028, 0x016c, NULL, NULL, "Dell", "PowerEdge 1850", intel_ich_gpio23_raise}, - {0x1039, 0x5513, 0x1019, 0x0A41, 0x1039, 0x0018, 0, 0, NULL, NULL, "Elitegroup", "K7S6A", elitegroup_k7vta3}, - {0x1106, 0x3038, 0x1019, 0x0996, 0x1106, 0x3177, 0x1019, 0x0996, NULL, NULL, "Elitegroup", "K7VTA3", elitegroup_k7vta3}, - {0x1106, 0x3177, 0x1106, 0x3177, 0x1106, 0x3059, 0x1695, 0x3005, NULL, NULL, "EPoX", "EP-8K5A2", w836xx_memw_enable_2e}, - {0x10EC, 0x8139, 0x1695, 0x9001, 0x11C1, 0x5811, 0x1695, 0x9015, NULL, NULL, "EPoX", "EP-8RDA3+", nvidia_mcp_gpio31_raise}, - {0x8086, 0x7110, 0, 0, 0x8086, 0x7190, 0, 0, "epox", "ep-bx3", "EPoX", "EP-BX3", board_epox_ep_bx3}, - {0x1039, 0x0761, 0, 0, 0x10EC, 0x8168, 0, 0, "gigabyte", "2761gxdk", "GIGABYTE", "GA-2761GXDK", it87xx_probe_spi_flash}, - {0x1106, 0x3227, 0x1458, 0x5001, 0x10ec, 0x8139, 0x1458, 0xe000, NULL, NULL, "GIGABYTE", "GA-7VT600", it8705_rom_write_enable}, - {0x10DE, 0x0050, 0x1458, 0x0C11, 0x10DE, 0x005e, 0x1458, 0x5000, NULL, NULL, "GIGABYTE", "GA-K8N-SLI", nvidia_mcp_gpio21_raise}, - {0x10DE, 0x0360, 0x1458, 0x0C11, 0x10DE, 0x0369, 0x1458, 0x5001, "gigabyte", "m57sli", "GIGABYTE", "GA-M57SLI-S4", it87xx_probe_spi_flash}, - {0x10de, 0x03e0, 0, 0, 0x10DE, 0x03D0, 0, 0, NULL, NULL, "GIGABYTE", "GA-M61P-S3", it87xx_probe_spi_flash}, - {0x1002, 0x4398, 0x1458, 0x5004, 0x1002, 0x4391, 0x1458, 0xb000, NULL, NULL, "GIGABYTE", "GA-MA78G-DS3H", it87xx_probe_spi_flash}, - {0x1002, 0x4398, 0x1458, 0x5004, 0x1002, 0x4391, 0x1458, 0xb002, NULL, NULL, "GIGABYTE", "GA-MA78GM-S2H", it87xx_probe_spi_flash}, - {0x1002, 0x438d, 0x1458, 0x5001, 0x1002, 0x5956, 0x1002, 0x5956, NULL, NULL, "GIGABYTE", "GA-MA790FX-DQ6", it87xx_probe_spi_flash}, - {0x1166, 0x0223, 0x103c, 0x320d, 0x102b, 0x0522, 0x103c, 0x31fa, "hp", "dl145_g3", "HP", "DL145 G3", board_hp_dl145_g3_enable}, - {0x1166, 0x0205, 0x1014, 0x0347, 0x1002, 0x515E, 0x1014, 0x0325, NULL, NULL, "IBM", "x3455", board_ibm_x3455}, - {0x1039, 0x5513, 0x8086, 0xd61f, 0x1039, 0x6330, 0x8086, 0xd61f, NULL, NULL, "Intel", "D201GLY", wbsio_check_for_spi}, - {0x1022, 0x7468, 0, 0, 0, 0, 0, 0, "iwill", "dk8_htx", "IWILL", "DK8-HTX", w83627hf_gpio24_raise_2e}, - {0x8086, 0x27A0, 0, 0, 0x8086, 0x27b8, 0, 0, "kontron", "986lcd-m", "Kontron", "986LCD-M", board_kontron_986lcd_m}, - {0x8086, 0x2411, 0x8086, 0x2411, 0x8086, 0x7125, 0x0e11, 0xb165, NULL, NULL, "Mitac", "6513WU", board_mitac_6513wu}, - {0x13f6, 0x0111, 0x1462, 0x5900, 0x1106, 0x3177, 0x1106, 0, NULL, NULL, "MSI", "MS-6590 (KT4 Ultra)",board_msi_kt4v}, - {0x1106, 0x3149, 0x1462, 0x7094, 0x10ec, 0x8167, 0x1462, 0x094c, NULL, NULL, "MSI", "MS-6702E (K8T Neo2-F)",w83627thf_gpio4_4_raise_2e}, - {0x1106, 0x0571, 0x1462, 0x7120, 0x1106, 0x3065, 0x1462, 0x7120, NULL, NULL, "MSI", "MS-6712 (KT4V)", board_msi_kt4v}, - {0x1039, 0x7012, 0x1462, 0x0050, 0x1039, 0x6325, 0x1462, 0x0058, NULL, NULL, "MSI", "MS-7005 (651M-L)", board_msi_651ml}, - {0x8086, 0x2658, 0x1462, 0x7046, 0x1106, 0x3044, 0x1462, 0x046d, NULL, NULL, "MSI", "MS-7046", intel_ich_gpio19_raise}, - {0x10DE, 0x005E, 0x1462, 0x7135, 0x10DE, 0x0050, 0x1462, 0x7135, "msi", "k8n-neo3", "MSI", "MS-7135 (K8N Neo3)", w83627thf_gpio4_4_raise_4e}, - {0x10DE, 0x005E, 0x1462, 0x7125, 0x10DE, 0x0052, 0x1462, 0x7125, NULL, NULL, "MSI", "K8N Neo4-F", nvidia_mcp_gpio2_raise}, - {0x1106, 0x3099, 0, 0, 0x1106, 0x3074, 0, 0, "shuttle", "ak31", "Shuttle", "AK31", w836xx_memw_enable_2e}, - {0x1106, 0x3104, 0x1297, 0xa238, 0x1106, 0x3059, 0x1297, 0xc063, NULL, NULL, "Shuttle", "AK38N", shuttle_ak38n}, - {0x10DE, 0x0050, 0x1297, 0x5036, 0x1412, 0x1724, 0x1297, 0x5036, NULL, NULL, "Shuttle", "FN25", board_shuttle_fn25}, - {0x1106, 0x3038, 0x0925, 0x1234, 0x1106, 0x3058, 0x15DD, 0x7609, NULL, NULL, "Soyo", "SY-7VCA", board_soyo_sy_7vca}, - {0x8086, 0x1076, 0x8086, 0x1176, 0x1106, 0x3059, 0x10f1, 0x2498, NULL, NULL, "Tyan", "S2498 (Tomcat K7M)", w836xx_memw_enable_2e}, - {0x1106, 0x3177, 0x1106, 0xAA01, 0x1106, 0x3123, 0x1106, 0xAA01, NULL, NULL, "VIA", "EPIA M/MII/...", via_vt823x_gpio15_raise}, - {0x1106, 0x0259, 0x1106, 0x3227, 0x1106, 0x3065, 0x1106, 0x3149, NULL, NULL, "VIA", "EPIA-N/NL", via_vt823x_gpio9_raise}, - {0x1106, 0x5337, 0x1458, 0xb003, 0x1106, 0x287e, 0x1106, 0x337e, NULL, NULL, "VIA", "PC3500G", it87xx_probe_spi_flash}, - - { 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL, NULL}, /* end marker */ + /* first pci-id set [4], second pci-id set [4], dmi identifier coreboot id [2], vendor name board name flash enable */ + {0x8086, 0x2926, 0x147b, 0x1084, 0x11ab, 0x4364, 0x147b, 0x1084, NULL, NULL, NULL, "Abit", "IP35", intel_ich_gpio16_raise}, + {0x105a, 0x0d30, 0x105a, 0x4d33, 0x8086, 0x1130, 0x8086, 0, NULL, NULL, NULL, "Acorp", "6A815EPD", board_acorp_6a815epd}, + {0x8086, 0x24D4, 0x1849, 0x24D0, 0x8086, 0x24D5, 0x1849, 0x9739, NULL, NULL, NULL, "ASRock", "P4i65GV", intel_ich_gpio23_raise}, + {0x1022, 0x746B, 0, 0, 0, 0, 0, 0, NULL, "AGAMI", "ARUMA", "agami", "Aruma", w83627hf_gpio24_raise_2e}, + {0x1106, 0x3177, 0x17F2, 0x3177, 0x1106, 0x3148, 0x17F2, 0x3148, NULL, NULL, NULL, "Albatron", "PM266A", w836xx_memw_enable_2e}, + {0x1106, 0x3205, 0x1106, 0x3205, 0x10EC, 0x8139, 0xA0A0, 0x0477, NULL, NULL, NULL, "AOpen", "vKM400Am-S", it8705_rom_write_enable}, + {0x1022, 0x2090, 0, 0, 0x1022, 0x2080, 0, 0, NULL, "artecgroup", "dbe61", "Artec Group", "DBE61", board_artecgroup_dbe6x}, + {0x1022, 0x2090, 0, 0, 0x1022, 0x2080, 0, 0, NULL, "artecgroup", "dbe62", "Artec Group", "DBE62", board_artecgroup_dbe6x}, + {0x1106, 0x3189, 0x1043, 0x807F, 0x1106, 0x3065, 0x1043, 0x80ED, NULL, NULL, NULL, "ASUS", "A7V600-X", board_asus_a7v600x}, + {0x1106, 0x3189, 0x1043, 0x807F, 0x1106, 0x3177, 0x1043, 0x808C, NULL, NULL, NULL, "ASUS", "A7V8X", board_asus_a7v8x}, + {0x1106, 0x3177, 0x1043, 0x80A1, 0x1106, 0x3205, 0x1043, 0x8118, NULL, NULL, NULL, "ASUS", "A7V8X-MX SE", w836xx_memw_enable_2e}, + {0x1106, 0x1336, 0x1043, 0x80ed, 0x1106, 0x3288, 0x1043, 0x8249, NULL, NULL, NULL, "ASUS", "M2V-MX", via_vt823x_gpio5_raise}, + {0x8086, 0x1a30, 0x1043, 0x8070, 0x8086, 0x244b, 0x1043, 0x8028, NULL, NULL, NULL, "ASUS", "P4B266", intel_ich_gpio22_raise}, + {0x8086, 0x1A30, 0x1043, 0x8025, 0x8086, 0x244B, 0x104D, 0x80F0, NULL, NULL, NULL, "ASUS", "P4B266-LM", intel_ich_gpio21_raise}, + {0x8086, 0x2570, 0x1043, 0x80F2, 0x105A, 0x3373, 0x1043, 0x80F5, NULL, NULL, NULL, "ASUS", "P4P800-E Deluxe", intel_ich_gpio21_raise}, + {0x10B9, 0x1541, 0, 0, 0x10B9, 0x1533, 0, 0, "^P5A$", "asus", "p5a", "ASUS", "P5A", board_asus_p5a}, + {0x10DE, 0x0030, 0x1043, 0x818a, 0x8086, 0x100E, 0x1043, 0x80EE, NULL, NULL, NULL, "ASUS", "P5ND2-SLI Deluxe", nvidia_mcp_gpio10_raise}, + {0x1106, 0x3149, 0x1565, 0x3206, 0x1106, 0x3344, 0x1565, 0x1202, NULL, NULL, NULL, "Biostar", "P4M80-M4", it8705_rom_write_enable}, + {0x8086, 0x3590, 0x1028, 0x016c, 0x1000, 0x0030, 0x1028, 0x016c, NULL, NULL, NULL, "Dell", "PowerEdge 1850", intel_ich_gpio23_raise}, + {0x1039, 0x5513, 0x1019, 0x0A41, 0x1039, 0x0018, 0, 0, NULL, NULL, NULL, "Elitegroup", "K7S6A", elitegroup_k7vta3}, + {0x1106, 0x3038, 0x1019, 0x0996, 0x1106, 0x3177, 0x1019, 0x0996, NULL, NULL, NULL, "Elitegroup", "K7VTA3", elitegroup_k7vta3}, + {0x1106, 0x3177, 0x1106, 0x3177, 0x1106, 0x3059, 0x1695, 0x3005, NULL, NULL, NULL, "EPoX", "EP-8K5A2", w836xx_memw_enable_2e}, + {0x10EC, 0x8139, 0x1695, 0x9001, 0x11C1, 0x5811, 0x1695, 0x9015, NULL, NULL, NULL, "EPoX", "EP-8RDA3+", nvidia_mcp_gpio31_raise}, + {0x8086, 0x7110, 0, 0, 0x8086, 0x7190, 0, 0, NULL, "epox", "ep-bx3", "EPoX", "EP-BX3", board_epox_ep_bx3}, + {0x1039, 0x0761, 0, 0, 0x10EC, 0x8168, 0, 0, NULL, "gigabyte", "2761gxdk", "GIGABYTE", "GA-2761GXDK", it87xx_probe_spi_flash}, + {0x1106, 0x3227, 0x1458, 0x5001, 0x10ec, 0x8139, 0x1458, 0xe000, NULL, NULL, NULL, "GIGABYTE", "GA-7VT600", it8705_rom_write_enable}, + {0x10DE, 0x0050, 0x1458, 0x0C11, 0x10DE, 0x005e, 0x1458, 0x5000, NULL, NULL, NULL, "GIGABYTE", "GA-K8N-SLI", nvidia_mcp_gpio21_raise}, + {0x10DE, 0x0360, 0x1458, 0x0C11, 0x10DE, 0x0369, 0x1458, 0x5001, NULL, "gigabyte", "m57sli", "GIGABYTE", "GA-M57SLI-S4", it87xx_probe_spi_flash}, + {0x10de, 0x03e0, 0, 0, 0x10DE, 0x03D0, 0, 0, NULL, NULL, NULL, "GIGABYTE", "GA-M61P-S3", it87xx_probe_spi_flash}, + {0x1002, 0x4398, 0x1458, 0x5004, 0x1002, 0x4391, 0x1458, 0xb000, NULL, NULL, NULL, "GIGABYTE", "GA-MA78G-DS3H", it87xx_probe_spi_flash}, + {0x1002, 0x4398, 0x1458, 0x5004, 0x1002, 0x4391, 0x1458, 0xb002, NULL, NULL, NULL, "GIGABYTE", "GA-MA78GM-S2H", it87xx_probe_spi_flash}, + {0x1002, 0x438d, 0x1458, 0x5001, 0x1002, 0x5956, 0x1002, 0x5956, NULL, NULL, NULL, "GIGABYTE", "GA-MA790FX-DQ6", it87xx_probe_spi_flash}, + {0x1166, 0x0223, 0x103c, 0x320d, 0x102b, 0x0522, 0x103c, 0x31fa, NULL, "hp", "dl145_g3", "HP", "DL145 G3", board_hp_dl145_g3_enable}, + {0x1166, 0x0205, 0x1014, 0x0347, 0x1002, 0x515E, 0x1014, 0x0325, NULL, NULL, NULL, "IBM", "x3455", board_ibm_x3455}, + {0x1039, 0x5513, 0x8086, 0xd61f, 0x1039, 0x6330, 0x8086, 0xd61f, NULL, NULL, NULL, "Intel", "D201GLY", wbsio_check_for_spi}, + {0x1022, 0x7468, 0, 0, 0, 0, 0, 0, NULL, "iwill", "dk8_htx", "IWILL", "DK8-HTX", w83627hf_gpio24_raise_2e}, + {0x8086, 0x27A0, 0, 0, 0x8086, 0x27b8, 0, 0, NULL, "kontron", "986lcd-m", "Kontron", "986LCD-M", board_kontron_986lcd_m}, + {0x8086, 0x2411, 0x8086, 0x2411, 0x8086, 0x7125, 0x0e11, 0xb165, NULL, NULL, NULL, "Mitac", "6513WU", board_mitac_6513wu}, + {0x13f6, 0x0111, 0x1462, 0x5900, 0x1106, 0x3177, 0x1106, 0, NULL, NULL, NULL, "MSI", "MS-6590 (KT4 Ultra)",board_msi_kt4v}, + {0x1106, 0x3149, 0x1462, 0x7094, 0x10ec, 0x8167, 0x1462, 0x094c, NULL, NULL, NULL, "MSI", "MS-6702E (K8T Neo2-F)",w83627thf_gpio4_4_raise_2e}, + {0x1106, 0x0571, 0x1462, 0x7120, 0x1106, 0x3065, 0x1462, 0x7120, NULL, NULL, NULL, "MSI", "MS-6712 (KT4V)", board_msi_kt4v}, + {0x1039, 0x7012, 0x1462, 0x0050, 0x1039, 0x6325, 0x1462, 0x0058, NULL, NULL, NULL, "MSI", "MS-7005 (651M-L)", board_msi_651ml}, + {0x8086, 0x2658, 0x1462, 0x7046, 0x1106, 0x3044, 0x1462, 0x046d, NULL, NULL, NULL, "MSI", "MS-7046", intel_ich_gpio19_raise}, + {0x10DE, 0x005E, 0x1462, 0x7135, 0x10DE, 0x0050, 0x1462, 0x7135, NULL, "msi", "k8n-neo3", "MSI", "MS-7135 (K8N Neo3)", w83627thf_gpio4_4_raise_4e}, + {0x10DE, 0x005E, 0x1462, 0x7125, 0x10DE, 0x0052, 0x1462, 0x7125, NULL, NULL, NULL, "MSI", "K8N Neo4-F", nvidia_mcp_gpio2_raise}, + {0x1106, 0x3099, 0, 0, 0x1106, 0x3074, 0, 0, NULL, "shuttle", "ak31", "Shuttle", "AK31", w836xx_memw_enable_2e}, + {0x1106, 0x3104, 0x1297, 0xa238, 0x1106, 0x3059, 0x1297, 0xc063, NULL, NULL, NULL, "Shuttle", "AK38N", shuttle_ak38n}, + {0x10DE, 0x0050, 0x1297, 0x5036, 0x1412, 0x1724, 0x1297, 0x5036, NULL, NULL, NULL, "Shuttle", "FN25", board_shuttle_fn25}, + {0x1106, 0x3038, 0x0925, 0x1234, 0x1106, 0x3058, 0x15DD, 0x7609, NULL, NULL, NULL, "Soyo", "SY-7VCA", board_soyo_sy_7vca}, + {0x8086, 0x1076, 0x8086, 0x1176, 0x1106, 0x3059, 0x10f1, 0x2498, NULL, NULL, NULL, "Tyan", "S2498 (Tomcat K7M)", w836xx_memw_enable_2e}, + {0x1106, 0x3177, 0x1106, 0xAA01, 0x1106, 0x3123, 0x1106, 0xAA01, NULL, NULL, NULL, "VIA", "EPIA M/MII/...", via_vt823x_gpio15_raise}, + {0x1106, 0x0259, 0x1106, 0x3227, 0x1106, 0x3065, 0x1106, 0x3149, NULL, NULL, NULL, "VIA", "EPIA-N/NL", via_vt823x_gpio9_raise}, + {0x1106, 0x5337, 0x1458, 0xb003, 0x1106, 0x287e, 0x1106, 0x337e, NULL, NULL, NULL, "VIA", "PC3500G", it87xx_probe_spi_flash}, + + { 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL, NULL, NULL}, /* end marker */ };
/** @@ -1346,6 +1353,18 @@ static struct board_pciid_enable *board_match_pci_card_ids(void) } }
+ if (board->dmi_pattern) { + if (!has_dmi_support) { + fprintf(stderr, "WARNING: Can't autodetect %s %s," + " DMI info unavailable.\n", + board->vendor_name, board->board_name); + continue; + } else { + if (!dmi_match(board->dmi_pattern)) + continue; + } + } + return board; }
diff --git a/dmi.c b/dmi.c new file mode 100644 index 0000000..ca75461 --- /dev/null +++ b/dmi.c @@ -0,0 +1,168 @@ +/* + * This file is part of the flashrom project. + * + * Copyright (C) 2009,2010 Michael Karcher + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include <string.h> +#include <stdio.h> +#include <stdlib.h> + +#include "flash.h" + +enum dmi_strings { + DMI_SYS_MANUFACTURER, + DMI_SYS_PRODUCT, + DMI_SYS_VERSION, + DMI_BB_MANUFACTURER, + DMI_BB_PRODUCT, + DMI_BB_VERSION, + DMI_ID_INVALID /* This must always be the last entry */ +}; + +/* The short_id for baseboard starts with "m" as in mainboard to leave + "b" available for BIOS */ +struct { + const char *dmidecode_name; + char short_id[3]; +} dmi_properties[DMI_ID_INVALID] = { + {"system-manufacturer", "sm"}, + {"system-product-name", "sp"}, + {"system-version", "sv"}, + {"baseboard-manufacturer", "mm"}, + {"baseboard-product-name", "mp"}, + {"baseboard-version", "mv"} +}; + +#define DMI_COMMAND_LEN_MAX 260 +const char *dmidecode_command = "dmidecode"; + +int has_dmi_support = 0; +char *dmistrings[DMI_ID_INVALID]; + +/* strings longer than 4096 in DMI are just insane */ +#define DMI_MAX_ANSWER_LEN 4096 + +void dmi_init(void) +{ + FILE *dmidecode_pipe; + int i; + char *answerbuf = malloc(DMI_MAX_ANSWER_LEN); + if(!answerbuf) + { + fprintf(stderr, "DMI: couldn't alloc answer buffer\n"); + return; + } + for (i = 0; i < DMI_ID_INVALID; i++) + { + char commandline[DMI_COMMAND_LEN_MAX+40]; + snprintf(commandline, sizeof(commandline), + "%s -s %s", dmidecode_command, + dmi_properties[i].dmidecode_name); + dmidecode_pipe = popen(commandline, "r"); + if (!dmidecode_pipe) + { + printf_debug("DMI pipe open error\n"); + goto out_free; + } + fgets(answerbuf, DMI_MAX_ANSWER_LEN, dmidecode_pipe); + if (ferror(dmidecode_pipe)) + { + printf_debug("DMI pipe read error\n"); + pclose(dmidecode_pipe); + goto out_free; + } + /* Toss all output above DMI_MAX_ANSWER_LEN away to prevent + deadlock on pclose. */ + while (!feof(dmidecode_pipe)) + getc(dmidecode_pipe); + if (pclose(dmidecode_pipe) != 0) + { + printf_debug("DMI pipe close error\n"); + goto out_free; + } + + /* chomp trailing newline */ + if (answerbuf[0] != 0 && + answerbuf[strlen(answerbuf) - 1] == '\n') + answerbuf[strlen(answerbuf) - 1] = 0; + printf_debug("DMI string %d: "%s"\n", i, answerbuf); + dmistrings[i] = strdup(answerbuf); + } + has_dmi_support = 1; +out_free: + free(answerbuf); +} + +/** + * Does an substring/prefix/postfix/whole-string match. + * + * The pattern is matched as-is. The only metacharacters supported are '^' + * at the beginning and '$' at the end. So you can look for "^prefix", + * "suffix$", "substring" or "^complete string$". + * + * @param value The string to check. + * @param pattern The pattern. + * @return Nonzero if pattern matches. + */ +static int dmi_compare(const char *value, const char *pattern) +{ + int anchored = 0; + int patternlen; + printf_debug("matching %s against %s\n", value, pattern); + /* The empty string is part of all strings */ + if (pattern[0] == 0) + return 1; + + if (pattern[0] == '^') { + anchored = 1; + pattern++; + } + + patternlen = strlen(pattern); + if (pattern[patternlen - 1] == '$') { + int valuelen = strlen(value); + patternlen--; + if(patternlen > valuelen) + return 0; + + /* full string match: require same length */ + if(anchored && (valuelen != patternlen)) + return 0; + + /* start character to make ends match */ + value += valuelen - patternlen; + anchored = 1; + } + + if (anchored) + return strncmp(value, pattern, patternlen) == 0; + else + return strstr(value, pattern) != NULL; +} + +int dmi_match(const char *pattern) +{ + int i; + if (!has_dmi_support) + return 0; + + for (i = 0;i < DMI_ID_INVALID; i++) + return dmi_compare(dmistrings[i], pattern); + + return 0; +} diff --git a/flash.h b/flash.h index 9a25c70..465feeb 100644 --- a/flash.h +++ b/flash.h @@ -262,6 +262,9 @@ struct board_pciid_enable { uint16_t second_card_vendor; uint16_t second_card_device;
+ /* Pattern to match DMI entries */ + const char *dmi_pattern; + /* The vendor / part name from the coreboot table. */ const char *lb_vendor; const char *lb_part; @@ -340,6 +343,11 @@ int coreboot_init(void); extern char *lb_part, *lb_vendor; extern int partvendor_from_cbtable;
+/* dmi.c */ +extern int has_dmi_support; +void dmi_init(void); +int dmi_match(const char *pattern); + /* internal.c */ #if NEED_PCI == 1 struct superio { diff --git a/internal.c b/internal.c index acb99b7..935240d 100644 --- a/internal.c +++ b/internal.c @@ -152,6 +152,7 @@ int internal_init(void) * mainboard specific flash enable sequence. */ coreboot_init(); + dmi_init();
/* Probe for the SuperI/O chip and fill global struct superio. */ probe_superio();
-
Carl-Daniel Hailfinger -
Luc Verhaegen -
Michael Karcher