[flashrom] DMI matching patch

[Carl-Daniel Hailfinger]

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 at 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

-- 
Developer quote of the year:
"We are juggling too many chainsaws and flaming arrows and tigers."