2010/8/9 Carl-Daniel Hailfinger <[email protected]> > Thanks for updating the patch. > Just a few short comments, this is not a full review. > > On 09.08.2010 01:05, Sean Nelson wrote: > > Here is a new version of the internal dmi decoding. To use the new > > decoder, the user needs to use "-p internal:dmi=new", to cross-check > > with the external dmidecode use "-p internal:dmi=check". Hopefully if > > this is comitted, it will make it easier for people to test the new > > internal decoder. > > > > Signed-off-by: Sean Nelson <[email protected]> > > > > diff --git a/dmi.c b/dmi.c > > index f18907f..b2a5daa 100644 > > --- a/dmi.c > > +++ b/dmi.c > > @@ -1,76 +1,227 @@ > > /* > > * This file is part of the flashrom project. > > * > > * Copyright (C) 2009,2010 Michael Karcher > > + * Copyright (C) 2010 Sean Nelson > > * > > * 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 <unistd.h> > > +#include <fcntl.h> > > +#include <sys/mman.h> > > > > Will this work on DOS and Windows? >
DOS: flashrom v0.9.2-r1136 on FreeDOS 7 (i786), built with libpci 3.1.5, GCC 4.3.2, little endian flashrom is free software, get the source code at http://www.flashrom.org Calibrating delay loop... OS timer resolution is 170000 usecs, 1873M loops per second, 10 myus = 0 us, 100 myus = 0 us, 1000 myus = 0 us, 10000 myus = 0 us, 680000 myus = 600000 us, OK. Initializing internal programmer No coreboot table found. DMI string system-manufacturer: "" DMI string system-product-name: "" DMI string system-version: "" DMI string baseboard-manufacturer: "" DMI string baseboard-product-name: "" DMI string baseboard-version: "" DMI string chassis-type: "" DMI string chassis-type: "" No DMI table found. Linux (Arch Linux): flashrom v0.9.2-r1136 on Linux 2.6.34-ARCH (i686), built with libpci 3.1.7, GCC 4.5.0 20100610 (prerelease), little endi an flashrom is free software, get the source code at http://www.flashrom.org Calibrating delay loop... OS timer resolution is 1 usecs, 2002M loops per second, 10 myus = 10 us, 100 myus = 100 us, 10 00 myus = 1136 us, 10000 myus = 10275 us, 4 myus = 5 us, OK. Initializing internal programmer No coreboot table found. sh: dmidecode: command not found dmidecode execution unsucessfull - continuing without DMI info FreeBSD: flashrom v0.9.2-r1136 on FreeBSD 8.1-RELEASE (i386), built with libpci 3.1.7, GCC 4.2.1 20070719 [FreeBSD], little endian flashrom is free software, get the source code at http://www.flashrom.org Calibrating delay loop... OS timer resolution is 6 usecs, 1973M loops per second, 10 myus = 11 us, 100 myus = 112 us, 1000 myus = 1005 us, 10000 myus = 9742 us, 24 myus = 48 us, OK. Initializing internal programmer No coreboot table found. DMI string system-manufacturer: " " DMI string system-product-name: "P4i65GV" DMI string system-version: "1.00" DMI string baseboard-manufacturer: " " DMI string baseboard-product-name: "P4i65GV" DMI string baseboard-version: "1.0" DMI string chassis-type: "" DMI string chassis-type: "" No DMI table found. > > > > + > > #include "flash.h" > > #include "programmer.h" > > > > int has_dmi_support = 0; > > > > #if STANDALONE > > > > Hm. STANDALONE should activate the builtin decoder. We might elect to > have a separate CONFIG_DMI which would replace !STANDALONE in this file. > > > > > > /* Stub to indicate missing DMI functionality. > > * has_dmi_support is 0 by default, so nothing to do here. > > * Because dmidecode is not available on all systems, the goal is to > implement > > * the DMI subset we need directly in this file. > > */ > > void dmi_init(void) > > { > > } > > > > int dmi_match(const char *pattern) > > { > > return 0; > > } > > > > #else /* STANDALONE */ > > > > -static const char *dmidecode_names[] = { > > - "system-manufacturer", > > - "system-product-name", > > - "system-version", > > - "baseboard-manufacturer", > > - "baseboard-product-name", > > - "baseboard-version", > > +/* the actual code, based on the dmidecode parsing logic */ > > +static const struct string_keyword { > > > > Kill string_keyword. > > > > + const char *keyword; > > + unsigned char type; > > + unsigned char offset; > > +} flashrom_dmi_strings[] = { > > + { "system-manufacturer", 1, 0x04 }, > > + { "system-product-name", 1, 0x05 }, > > + { "system-version", 1, 0x06 }, > > + { "baseboard-manufacturer", 2, 0x04 }, > > + { "baseboard-product-name", 2, 0x05 }, > > + { "baseboard-version", 2, 0x06 }, > > + { "chassis-type", 3, 0x05 }, > > }; > > > > #define DMI_COMMAND_LEN_MAX 260 > > static const char *dmidecode_command = "dmidecode"; > > +static char *external_dmi[ARRAY_SIZE(flashrom_dmi_strings)]; > > +#define DMI_MAX_ANSWER_LEN 4096 > > > > -static char *dmistrings[ARRAY_SIZE(dmidecode_names)]; > > +static char *dmistrings[ARRAY_SIZE(flashrom_dmi_strings)]; > > > > -/* Strings longer than 4096 in DMI are just insane. */ > > > > Why did the comment disappear? > > > > -#define DMI_MAX_ANSWER_LEN 4096 > > +#define WORD(x) (unsigned short)(*(const unsigned short *)(x)) > > +#define DWORD(x) (unsigned int)(*(const unsigned int *)(x)) > > > > Ugh. Those #defines are really ugly. Can't you use mmio_read[bwl] or > something similar? > > > > + > > +static int checksum(const unsigned char *buf, size_t len) > > > > dmi_checksum? > > > > +{ > > + unsigned char sum = 0; > > + size_t a; > > + > > + for (a = 0; a < len; a++) > > + sum += buf[a]; > > + return (sum == 0); > > +} > > + > > +static char *dmi_string(char *bp, unsigned char length, unsigned char s) > > > > Better names for bp and s would be appreciated unless this is a straight > copy from the original dmidecode code (in which case we'd have to add > the author to the copyright header). > > > > +{ > > + size_t i, len; > > + > > + if (s == 0) > > + return "Not Specified"; > > + > > + bp += length; > > + while (s > 1 && *bp) { > > + bp += strlen(bp); > > + bp++; > > > > We happily walk off the cliff here (mapped area) if the DMI strings > contain garbage. > > > > + s--; > > + } > > + > > + if (!*bp) > > + return "<BAD INDEX>"; > > + > > + len = strlen(bp); > > + for (i = 0; i < len; i++) > > + if (bp[i] < 32 || bp[i] == 127) > > + bp[i] = '.'; > > > > We write to bp? > > > > + > > + return bp; > > +} > > + > > +static int dmi_chassis_type(unsigned char code) > > +{ > > + switch(code) { > > + case 0x08: /* Portable */ > > + case 0x09: /* Laptop */ > > + case 0x0A: /* Notebook */ > > + case 0x0E: /* Sub Notebook */ > > + return 1; > > + default: > > + return 0; > > + } > > +} > > + > > +static void dmi_table(unsigned int base, unsigned short len, unsigned > short num) > > +{ > > + unsigned char *data; > > + unsigned char *dmi_table_mem; > > + int key; > > + int i=0, j=0; > > > > spaces > > > > + > > + dmi_table_mem = physmap_try_ro("DMI Tables", base, len); > > > > If physmap_try_ro fails this will explode spectacularly. > > > > + data = dmi_table_mem; > > + > > + /* 4 is the length of an SMBIOS structure header */ > > + while (i < num && data+4 <= dmi_table_mem + len) { > > + unsigned char *next; > > + /* > > + * If a short entry is found (less than 4 bytes), not only > it > > + * is invalid, but we cannot reliably locate the next > entry. > > + * Better stop at this point, and let the user know his/her > > + * table is broken. > > + */ > > + if (data[1] < 4) { > > + msg_perr("Invalid entry length (%u). DMI table is " > > + "broken! Stop.\n\n", (unsigned > int)data[1]); > > + break; > > + } > > + > > + /* Stop decoding after chassis segment */ > > + if (data[0] == 4) > > + break; > > + > > + /* look for the next handle */ > > + next = data + data[1]; > > + while (next - dmi_table_mem + 1 < len && (next[0] != 0 || > next[1] != 0)) > > + next++; > > + next += 2; > > + > > + for (j = 0; j < ARRAY_SIZE(flashrom_dmi_strings); j++) > > + { > > + unsigned char offset = > flashrom_dmi_strings[j].offset; > > + unsigned char type = flashrom_dmi_strings[j].type; > > + > > + if (offset >= data[1]) > > + return; > > + > > + key = (type << 8) | offset; > > + > > + switch (key) > > + { > > + case 0x305: /* detect if laptop */ > > + if (dmi_chassis_type(data[offset])) { > > + msg_pdbg("Laptop detected via > DMI\n"); > > > > Can you print the chassis type in hex as well, even if it is not a > laptop? This will be helpful if a vendor screws up the DMI tables. > > > > + is_laptop = 1; > > + } > > + break; > > + default: > > + if (type == data[0]) { > > + dmistrings[j] = > dmi_string((char*)data, data[1], data[offset]); > > + msg_pinfo("DMI string %s: > \"%s\"\n", > > + > flashrom_dmi_strings[j].keyword, dmistrings[j]); > > + } > > + } > > + } > > + data = next; > > + i++; > > + } > > + > > + physunmap(dmi_table, len); > > +} > > + > > +static int smbios_decode(unsigned char *buf) > > +{ > > + if (!checksum(buf, buf[0x05]) > > + || (memcmp(buf + 0x10, "_DMI_", 5) != 0) > > + || !checksum(buf + 0x10, 0x0F)) > > + return 0; > > + > > + dmi_table(DWORD(buf + 0x18), WORD(buf + 0x16), WORD(buf + 0x1C)); > > + > > + return 1; > > +} > > + > > +static int legacy_decode(unsigned char *buf) > > +{ > > + if (!checksum(buf, 0x0F)) > > + return 0; > > + > > + dmi_table(DWORD(buf + 0x08), WORD(buf + 0x06), WORD(buf + 0x0C)); > > + > > + return 1; > > +} > > > > static char *get_dmi_string(const char *string_name) > > { > > FILE *dmidecode_pipe; > > char *result; > > char answerbuf[DMI_MAX_ANSWER_LEN]; > > char commandline[DMI_COMMAND_LEN_MAX + 40]; > > > > snprintf(commandline, sizeof(commandline), > > "%s -s %s", dmidecode_command, string_name); > > dmidecode_pipe = popen(commandline, "r"); > > if (!dmidecode_pipe) { > > msg_perr("DMI pipe open error\n"); > > @@ -109,45 +260,111 @@ static char *get_dmi_string(const char > *string_name) > > answerbuf[strlen(answerbuf) - 1] == '\n') > > answerbuf[strlen(answerbuf) - 1] = 0; > > msg_pdbg("DMI string %s: \"%s\"\n", string_name, answerbuf); > > > > result = strdup(answerbuf); > > if (!result) > > puts("WARNING: Out of memory - DMI support fails"); > > > > return result; > > } > > > > void dmi_init(void) > > { > > - int i; > > - char *chassis_type; > > + int found = 0; > > + size_t fp; > > + unsigned char *dmi_mem = NULL; > > + char *arg = NULL; > > + int use_new_dmi = 0; > > + int i = 0; > > + > > + arg = extract_programmer_param("dmi"); > > + if (arg && !strcmp(arg,"new")) { > > + use_new_dmi = 1; > > + } else if (arg && !strcmp(arg, "check")) { > > + use_new_dmi = 2; > > + } else if (arg && !strlen(arg)) { > > + msg_perr("Missing argument for dmi. Falling back to > external dmi\n"); > > + } else if (arg) { > > + msg_perr("Unknown argument for dmi: %s. Falling back to > external dmi\n", arg); > > > > You could also return here. If the user doesn't know what he wants, we > shouldn't continue. > > > > + } > > + free(arg); > > > > has_dmi_support = 1; > > - for (i = 0; i < ARRAY_SIZE(dmidecode_names); i++) { > > - dmistrings[i] = get_dmi_string(dmidecode_names[i]); > > - if (!dmistrings[i]) { > > - has_dmi_support = 0; > > - return; > > + > > + if (use_new_dmi > 0) { > > + dmi_mem = physmap_try_ro("DMI", 0xF0000, 0x10000); > > + > > + if (!dmi_mem) > > + goto func_exit; > > + > > + for (fp = 0; fp <= 0xFFF0; fp += 16) { > > + if (memcmp(dmi_mem + fp, "_SM_", 4) == 0 && fp <= > 0xFFE0) { > > + if (smbios_decode(dmi_mem+fp)) { > > + found++; > > + fp += 16; > > + } > > + } > > + else if (memcmp(dmi_mem + fp, "_DMI_", 5) == 0) > > + if (legacy_decode(dmi_mem + fp)) > > + found++; > > } > > } > > > > - chassis_type = get_dmi_string("chassis-type"); > > - if (chassis_type && (!strcmp(chassis_type, "Notebook") || > > - !strcmp(chassis_type, "Portable"))) { > > - msg_pdbg("Laptop detected via DMI\n"); > > - is_laptop = 1; > > + if (use_new_dmi == 2) { > > + for (i = 0; i < ARRAY_SIZE(flashrom_dmi_strings); i++) > > + { > > + external_dmi[i] = > get_dmi_string(flashrom_dmi_strings[i].keyword); > > + if (!external_dmi[i]) > > + goto func_exit; > > + } > > + > > + for (i = 0; i < ARRAY_SIZE(flashrom_dmi_strings)-1; i++) > > > > spaces > > > > + { > > + if (strcmp(dmistrings[i], external_dmi[i])) > > + msg_perr("dmidecode vs internal-dmi > differs: %s\n", flashrom_dmi_strings[i].keyword); > > + else > > + msg_pspew("Matching of dmidecode and > internal-dmi succeeded!\n"); > > > > Hm. Do we cross-check the laptop info? > > > > + } > > } > > - free(chassis_type); > > + > > + if (use_new_dmi == 0) { > > + char *chassis_type; > > + has_dmi_support = 1; > > + for (i = 0; i < ARRAY_SIZE(flashrom_dmi_strings); i++) { > > + dmistrings[i] = > get_dmi_string(flashrom_dmi_strings[i].keyword); > > + if (!dmistrings[i]) { > > + has_dmi_support = 0; > > + return; > > + } > > + } > > + > > + chassis_type = get_dmi_string("chassis-type"); > > + if (chassis_type && (!strcmp(chassis_type, "Notebook") || > > + !strcmp(chassis_type, "Portable"))) { > > + msg_pdbg("Laptop detected via DMI\n"); > > + is_laptop = 1; > > + } > > + free(chassis_type); > > + } > > + > > +func_exit: > > + if (!found) > > + { > > + msg_pinfo("No DMI table found.\n"); > > + has_dmi_support = 0; > > + } > > + > > + physunmap(dmi_mem, 0x10000); > > } > > > > /** > > * 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. > > */ > > @@ -185,21 +402,21 @@ static int dmi_compare(const char *value, const > char *pattern) > > 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 < ARRAY_SIZE(dmidecode_names); i++) > > + for (i = 0; i < ARRAY_SIZE(flashrom_dmi_strings); i++) > > if (dmi_compare(dmistrings[i], pattern)) > > return 1; > > > > return 0; > > } > > > > #endif /* STANDALONE */ > > > > > > Regards, > Carl-Daniel > > -- > http://www.hailfinger.org/ > > > _______________________________________________ > flashrom mailing list > [email protected] > http://www.flashrom.org/mailman/listinfo/flashrom >
_______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
