Hi list, On Thu, Jun 27, 2024 at 1:45 PM Greg Troxel <g...@lexort.com> wrote: > > Anastasia Klimchuk <a...@chromium.org> writes: > > > * As the time goes, chip vendors are producing newer models, which > > sometimes re-use model IDs of old versions. Old versions are > > considered as "end of life", and replaced with newer models with more > > features. However flashchips.c accumulates everything and with time we > > get more and more duplicate model IDs. This makes adding new models > > harder, but also increases the chance users need to manually specify > > the chip model.
How exactly do duplicate IDs make adding new models harder? > Reusing IDs for things that are not the same is a really unfortunate > practice. I can't sew how that would lead to anyting but trouble, for > anybody's update code. I think it would be good for the project to > somehow ask the manufacturers not to do this, if that's comfortable. Unfortunately, I think there are not enough bits to use unique IDs everywhere. Plus, this would not address the problem existing flash chips. > > Question: How can we avoid old chip models dragging down adding > > new ones, without completely removing them from the database (some > > users might still be using end-of-life chips)? Newer chips support SFDP (Serial Flash Discoverable Parameter), a specification that allows (at least) SPI flash chips to self-document themselves to software, and that nearly all flash chips from the last decade or so support. So, I would highly recommend using SFDP to differentiate between flash chips with same ID but different capabilities, and thus rule out incompatible chip definitions. One can even use whether the probed flash chip supports SFDP to rule out entries (a flash chip that does not respond to SFDP should not be used with a chip definition of a SFDP-capable chip). Unfortunately, it looks like the flashchips database does not have a feature flag for SFDP (just a bunch of comments that may or may not be accurate). > This question seems very strange to me. The point of a program like > flashrom is to support the hardware that people have, and "end-of-life" > in chips usually seems like "we are not longer making this chip", not > "anyone who has a device containing this chip should immediately stop > using it and take it to recycling". > > The question should then be two > > Given that chip vendors have the buggy practice of reusing IDs for > different chips, what kind of UI and configuration is appropriate so > that flashrom users can use the program safely and effectively. IIRC, flashrom will refuse to perform any operations and ask users to specify a chip definition to use by adding a command-line parameter. One would then use the chip definition that matches the model written on the flash chip's package. But there are people who do not do that, and instead blindly choose the first chip definition that flashrom lists (even if it is wrong). A few others (although it is rather uncommon) copy only part of the chip definition's name, and flashrom promptly complains that it does not know about that chip definition. Which results in confused noises emanating from the user. > Are there chips whose IDs have been reused that we are willing to say > that essentially none of them are in service, so that flashrom can > just treat any with that ID as the new version? This is a combination > of "will doing this brick the old ones" and "how many are there". > This is really a secondary question; the main one is how to use both. I believe ID reusing became noticeably more common after SFDP got introduced, as SFDP provides significantly more detailed information about a flash chip's capabilities. Flash chips still have IDs because older software may rely on them. And yes, I am implying (and hereinafter explicitly stating) that flashrom is "old software": while it does incorporate SFDP support, it does not take advantage of SFDP to enhance matching against the flashchips database. > > * have a flag "end of life" for chip definition? by default, given > > multiple definitions of the same model ID, the definition without a > > flag be selected? > > I don't really see "end of life" as the real issue. I see it as "chip > vendor has shipped two kinds of chips with the same ID and we need to > invoke some further disambiguation mechanism before we operate on it". > > The fact that they did it because they stopped selling the other one, > rather than that they did it because they are just confused, is not > really relevant. The only thing that matters is that when flashrom > reads that ID, it does not know what chip it is dealing with. This. Even if one were to mark chip definitions for older chips as "end of life", there are more cases of ID reuse happening with *newer* chips that would not be dealt with. IMHO, this "end of life" flag would be like applying a band-aid to to someone who lost a loved one (it does not help, and it probably makes things worse). > > * vendors adding new model can mark old one end-of-life, in > > the same patch > > * [joursoir] It doesn't bother me. maybe it's not a big problem? > > but just having a flag "end-of-life" for information might be useful > > * need to be a good clear name, because flashrom still > > supports everything! > > * separate file flashchips_legacy.c for old chips? Now I wonder: if flashrom still supports everything, what would moving old chips to flashrom_legacy.c even achieve? > > * bad: moving definitions from one file to another is pita > > * CONCLUSION: maybe we can add a flag to flashchip struct and no > > changes to probing logic yet. > > * ticket: > > [https://ticket.coreboot.org/issues/542](https://ticket.coreboot.org/issues/542) > > This all feels off, reasoning from vendor eol being reasonable and > trying to record that, vs thinking about users and hardware likely to be > encountered. Given that there were only two people in the meeting, it is likely that things everyone agreed with were never questioned. Fortunately, posting the meeting minutes on the mailing list brought this to the attention of a much larger audience (*waves at the reader*). > What are you going to do with the eol flag? Consider a manufacturer > that stops selling a chip and thus it is EOL, but that they have good > management practices and do not reuse the ID. It seems like you should > set the flag. What then happens at runtime? Do you tell the user "the > flash chip in your computer is no longer manufactured"?? I would think > that almost all computers in use are no longer manufactured, unless they > are super new. Not to mention that there's cases of chips released around the same time (so none of them is a successor for the other one), all using the same ID but having different feature sets (which SFDP would describe). Age is pretty much irrelevant here, especially given that flashrom is to support everything (as per the meeting minutes). > The real issue is "there are two chips with this id", and this is not > about EOL. In that case, the chip structure needs to have a > flashrom_revision_code value to separate them, remediating the > manufacturer's choice to reuse, and the user needs to configure > > manufacturer_id: flashrom_revision_code > > in a config file so the match will lead to using the right definition. To me, the idea of a "flashrom revision code" sounds extremely cumbersome to maintain. Who assigns flashrom revision codes, for both existing chip definitions and new ones? Why would revision codes exist when there's chip definition names? I strongly believe that *the* solution is to leverage SFDP to enhance identification. However, there's a few other improvements that could be made to reduce the impact of "multiple flash chip definitions match". First of all, the chip definition names are what flashrom users use to choose a specific definition, but they are neither backward nor forward compatible. There's no guarantee that other flashrom versions will use the same name, so anyone using flashrom with a specific chip definition (`-C "CHIP_DEFINITION_NAME"`) in automated scripts needs to pin the flashrom version (require a specific version) to prevent things from potentially breaking in the future (when flashrom updates). My initial approach (what I've been thinking for the last minutes while writing this paragraph) is that chip definition names that comprise multiple chip models (some of them even use wildcards, and do so inconsistently) are mostly useless, so they have to go. Instead of those, I would have a list of "applicable" flash chip models, as they appear on the chip (with package type suffixes omitted, which is usually the case already). For example, instead of this: ``` .name = "W25Q32BV/W25Q32CV/W25Q32DV", ``` I would have something like this: ``` .names = { "W25Q32BV", "W25Q32CV", "W25Q32DV", }, ``` It is worth noting that the user shall not be asked to pick one the names of a chip definition when only said chip definition matches (one could simply print the result of joining the names with `/` as delimiter). When printing the possible chip definition choices, the names should be formatted as a list (of definitions) of lists (names of each definition). Of course, the above is just to show the idea. Doing this directly in C code will not work: the array in the structure would either need to have a fixed size (which is wasteful) or it would need to be defined as separate variables (which would be all the way at the top of the file, not next to the flash chip entries). Welp, more reasons to consider converting the flashchips database to use a data interchange format (e.g. JSON). Secondly, I can imagine a few situations where being able to "assume" certain chip definitions would work as a stop-gap measure. Consider the situation of someone who uses flashrom with a small number of different flash chips, all with distinct IDs, but some of which matching multiple chip definitions in flashrom. Because `-C` restricts detection to the supplied chip definition (can it even be specified multiple times?), they would need to change the `-C` part of the command line quite often, even if the rest of the command-line remains the same (e.g. `flashrom -p ft2232_spi --ifd -i bios -w build/coreboot.rom`). If they always use the same chip definition for a given chip ID (e.g. for the `WINBOND_NEX_W25Q32_V` ID, always use the "W25Q32JV" definition), it would be convenient to be able to use something like `flashrom -p ft2232_spi --ifd -i bios -w build/coreboot.rom --assume-chipdef "W25Q32JV"` (the `--assume-chipdef` parameter can appear multiple times). Unlike `-C`, this would work even if the detected chip is not a `WINBOND_NEX_W25Q32_V`. Note that the long name is intentional, as it makes flashrom assume something that might not be true (even though `-C` can already do the same). All of this reminds me: does flashrom still mindlessly try probing all chip definitions one by one? For SPI flash chips using one (and the same) probe function that returns numeric IDs (e.g. `probe_spi_rdid`), one could use these IDs to find matching chip definitions much faster than iterating over them. I think a multimap (or even a map where the values are arrays) would work quite nicely for this, but it would require acquiring an implementation of the data structure somehow (does the stdlib have one? probably not). Note that this would call for a restructuring of the flashchips database (although one could keep the existing database format for non-SPI flash chips, as those tend to use more chip-specific probing sequences). > _______________________________________________ > flashrom mailing list -- flashrom@flashrom.org > To unsubscribe send an email to flashrom-le...@flashrom.org TL;DR use SFDP. Best regards, Angel _______________________________________________ flashrom mailing list -- flashrom@flashrom.org To unsubscribe send an email to flashrom-le...@flashrom.org
