[flashrom] DMI matching patch

[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.