[flashrom] DMI matching patch

[Michael Karcher]

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