Patchworkβ internal dmi decoder

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 <audiohacked@gmail.com>

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 <audiohacked@gmail.com>
>
> 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?


> +
>  #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

2010/8/9 Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>

> 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 <audiohacked@gmail.com>
> >
> > 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
> flashrom@flashrom.org
> http://www.flashrom.org/mailman/listinfo/flashrom
>

Updated patch 2010-09-19. Uses its own copy of the dmi_chassis_types 
list. Each dmi_init prints if its either internal DMI or external 
Dmidecode. Selectable using -D'STANDALONE=1'

Signed-off-by: Sean Nelson <audiohacked@gmail.com>

On 20.09.2010 02:28, Sean Nelson wrote:
> Updated patch 2010-09-19. Uses its own copy of the dmi_chassis_types
> list. Each dmi_init prints if its either internal DMI or external
> Dmidecode. Selectable using -D'STANDALONE=1'
>
> Signed-off-by: Sean Nelson <audiohacked@gmail.com>

Looks like you forgot to attach the patch.

Regards,
Carl-Daniel