flashrom April 2022

flashrom@flashrom.org

24 participants
24 discussions

Flashrom Dev Meeting summary info
by Anastasia Klimchuk 18 May '22

18 May '22

Hello People, I wanted to summarise the information on the time of Dev meeting. At some point there was an idea to alternate times, but then we discovered that we got really lucky and there is one time which everyone agrees on. That was an open question at the last meeting, to confirm the time with those US folks who were not present. Since no one was replying to the mailing list, I reached out to folks and confirmed the time is fine. —------------- SUMMARY —------------- #1 Between November … [View More]

1 1

Release preparations
by Nico Huber 17 May '22

17 May '22

Hi folks, people often ask me about a potential next release and what is holding me back. Long story short, I knew about some bad things in our master branch that make a release impossible and I was very much afraid that there could be more that I don't know about. I've been scrolling through the commit log from v1.1 (the last release I was involved) to today's master. Some of the things I found are simple like something missing for a flash chip entry. But most of it are things I'd rather not … [View More]have on a release branch. And the list turned out to be much longer than I expected. OTOH, I didn't find issues of the worst kind I expected. Not sure how to go on from here. As this is a lot to do, I suggest that we freeze the master branch for everything that is neither a fix nor on the list (or a similar case that I missed). I noted sometimes that we could just drop the code from a release branch. If we'd decide to do that, we'd also need a plan how to stop the rot on the master branch. Below are my raw notes, I don't have the time to write things up nicely. I noted the commit hash where things began, but tried to take the cur- rent state into account. For instance, when it's about a new programmer driver, I was looking at the file, not the initial patch. Cheers, Nico Recommended clean-ups and investigations before v1.3 ==================================================== * cb973683 Added partial meson support which confuses package main- tainers. Should we remove it from or disable CLI building on release branches? * 71b706f5 Adds some libflashrom APIs with questionable design, buggy implementation and no visible user. Would be easiest to drop and add back when somebody needs it. Open comment. Partially fixed up. * 86fc9cf7 Open comment. Partially fixed up. * 05c629be Looks incomplete, flash would be fully erased with programmers that lack native 4BA command support. * ad08aef6 Undocumented programmer. disable? drop from realease branch? * 703de983 Added `spireadmode` param to sb600spi. (How) does it work? * 13a2ef6c Added `lspcon_i2c_spi`. Too generic name: LSPCon is a concept but driver seems to be about a specific vendor/chip. No pro- bing just starts to write to hardcoded addresses. WP code contains magic numbers, board specific? Implements `spi_ master` but contains hard-coded SPI opcodes. Not documented? disable? drop from release branch? * d97f87b0 Added `realtek_mst_i2c_spi`. No probing, starts by toggling a GPIO. Are GPIO settings board specific? Timeout checks seem to have regressed since original commit. Not documented? Changes clock settings, is this board specific? Implements `spi_master` with special cases in an opcode LUT, doesn't bail out on unknown codes. Falls back on unaligned reads/ writes, were these paths tested? Fixups suggest that the original code wasn't properly tested, was it by now? disable? drop from release branch? * be4682dc Might miss some block erasers. * f82dd300 Might miss some block erasers. * a2dc4f7f It seems `clear_spi_id_cache()` is never called. Probably, should be before probing. * 3149822c Added variable size in `dummy_programmer`. Didn't look at it right now, but I remember it caused a lot of trouble. Probably not fully fixed up yet. * cb9f3cd0 Possible division by zero. * ca2e3bce Fixed some libpci usage. IIRC, there are many more similar cases that might need a fix now that libpci is more strict. * 0386aa17 sb600spi refactoring. Looks like it dropped an error message from many error paths. * f745d0e6, adbae0e2 Added infrastructure for S25F chips. Code style looks a bit outdated. Probably needs upstream review. * 8fa792fb Added chips with oddities and w/o reasoning, e.g: .total_size = 16384, /* This is just half the size.... */ Complete patch needs review. * 855d6b6f Added Ryzen support to sb600spi. API violations. Stalled other cleanup work. Soft bricks machines with >16MiB flash. Patches on Gerrit. drop from release branch? * 32f4cb4f Looks like copy-pasta from Winbond chips. Might miss some block erasers. Missing 4BA_WRITE is probably wrong. * 45d50a10 Added support code for per-region file arguments. Review was interrupted, IIRC. Rebase was borked shortly before merge. Work never finished. * d4063bf3 Missed to update the manpage. Literal `-` shouldn't be in `<>`. * ce983bcc Missed to update the manpage. Filters only spaces when converting region names to file names. * 51e1d0e4 Still selects the wrong chipset, IIRC. * e98b2d11 Review never really started. See comments on Gerrit. Backport candidates =================== * 4ca575dc usbdev: Only match requested USB devices * 8bbb7648 spi95: Check for success before using send_command's returned data * b07c53d7 spi25: Debug flashrom crash when Write Protect is ON Looks like this covers up a general problem with flash-chip entries. Why would we know how to disable block protection if we don't know how to report it? Should probably be added to self checks. * 51261541 it87spi.c: Prevent use-after-free bug * da0825f0 chipset_enable.c: check return value from rphysmap() call * 705212da chipset_enable.c: Validate physmap() return rcrb value * b76d2810 dediprog: Fix segmentation fault on no device found * 1a21cc70 helpers.c: Fix undefined behavior in strndup() * a2f2f3f5 linux_mtd: Disable buffering on the mtd device * ab623c65 jlink_spi: Reduce transfer size * Many fixes in dummy_flasher, not sure if they were applicable earlier. * aedfb7eb flashrom.8: add missing entry for `--flash-contents` * 89c73b5a linux_mtd: check ioctl() return value properly [View Less]

11 25

Gatekeeping, ACLs and Review Rules
by Nico Huber 17 May '22

17 May '22

Dear fellow flashrom developers, I've been reflecting lately about written and unwritten rules around our review process and our related privileges on Gerrit. I couldn't identify anything that works well, or let's say well enough. So it might be time we discuss some things and make changes :) Before the dawn of our Git repository and using Gerrit as a review plat- form, we had a Subversion repository. With very few people, usually one or two I think, that had write access. So practically we … [View More]had gatekeepers that watched over the code. Eventually, they became a bottleneck so we widened that a little when starting the work on Gerrit. Today, I often catch myself falling back into such a gatekeeper mode. Which is futile anyway because I'm not the only one who can write to the repository. But I still wonder why this happens and if it's necessary. I guess first everyone needs to understand how we imagined things with Gerrit and how it looks like today. As mentioned above we set up a group "flashrom developers" which has exclusive write access to the reposi- tory. The idea was to only have people in this group who are long-term (I was thinking about years) contributors to flashrom, know each other from real-time communication (IRC) and trust each other. For the review right "+2", we used the "Reviewers" group from coreboot. IIRC, I pushed for the latter. I was hoping to grow the communities a little together that seemed to have gone separate ways in the years before. After all, a lot of coreboot developers use flashrom in their everyday work. Combined with the "flashrom developers" group it shouldn't have been an issue that people with little flashrom knowledge would review. Since the original setup (about 2017) things went on rather uncon- trolled. I think the gist of it is that people didn't expect that flashrom would need/have different rules compared to coreboot. At some point somebody whom I only knew from their work on coreboot sud- denly merged patches for flashrom. And I guess people pushing patches today pretty much expect that there is nothing more to be done once they see the first +2. No matter if the reviewer knows anything about flashrom. When reviewers disagree then, it can become frustrating for everyone. So currently, I lean towards aligning our process more with that of coreboot. The big difference seems to be that at coreboot merging patches, i.e. hitting the "Submit" button, is more or less considered an administrative task unrelated to one's opinion on the code. While for us it was supposed to replace the gatekeeping. Changing this would imply that we need our own group of reviewers. With this, a +2 would become both rarer and stronger. If this idea meets some consent, I would propose to copy the "flashrom developers" group for a start. And when we identify somebody over time who shows in-depth knowledge about at least a part of flashrom and good self-judgement if they are experienced enough for a +2, we could add them. Right now, some candidates come to mind already, Peter, Nikolai, and Thomas. The "flashrom developers" group also grants the privilege to block "-2" a change. I have very conflicting feelings about this. On one hand, this is a tool that is only ever necessary when people in said group can't trust each other. On the other, that seems to be the status quo already. Maybe we should have another group on top for this? This time really only for trusted, established, long-term contributors? Or maybe for those who maintain the codebase beyond what they are paid for? There is one related idea for a rule that crossed my mind: How about we forbid blocking reverts that fix a regression and apply cleanly (i.e. without needing to revert anything else in advance). Also, let them pass after review without waiting 24 hours. At least for myself this would reduce a lot pressure to fall back to gatekeeping if I knew I could keep things safe and sound without discussion. For maintainers, the patches that are already merged matter the most, cause the most work. This should also be the case for contributors, but alas many don't see it like this and don't help to fix things after a patch is merged. Easing reverts would push the hard work back to the people who break things. Looks a bit longer than what I intended to write, as usual when one starts writing ;) If we come to a conclusion wrt. Gerrit privileges, we could also discuss more rules for each role. While I'm not a fan of too many rules and would rather keep things to common sense, these things depend much on the people we add to the respective groups. I hope it's not too much to digest at once ;) Cheers, Nico [View Less]

5 17

Questions regarding Easy project and GSoC
by Aarya Chaumal 17 May '22

17 May '22

Hello there, I hope you are well today when you receive this email. I am Aarya Chaumal, a Computer Engineering student at the College of Engineering Pune, India. While going through organizations for this year's Google's Summer of Code I came across your organization, Flashrom. I am a part of onboard computers subsystem at my college's satellite initiative. Through this, I have closely worked on Atmel SAM E70 XPLAINED board. Also, I have strong knowledge about C/C++ and assembly language. … [View More]

7 34

MX25L12833F
by Jon Bowen 02 May '22

02 May '22

2 1

[flashrom - Bug #352] (New) test issue for mailing list
by Felix Singer 25 Apr '22

25 Apr '22

Issue #352 has been reported by Felix Singer. ---------------------------------------- Bug #352: test issue for mailing list https://ticket.coreboot.org/issues/352 * Author: Felix Singer * Status: New * Priority: Normal * Target version: none * Start date: 2022-04-25 ---------------------------------------- This is a test issue for the flashrom mailing list. -- You have received this notification because you have either subscribed to it, or are involved in it. To change your notification … [View More]

1 0

Unsupported chip AT25SF128A
by Staffan Ulfberg 23 Apr '22

23 Apr '22

Datasheet at: https://www.dialog-semiconductor.com/sites/default/files/2021-02/DS-AT25SF1… Probing for Unknown SFDP-capable chip, 0 kB: SFDP revision = 1.0 SFDP number of parameter headers is 2 (NPH = 1). SFDP parameter table header 0/1: ID 0x00, version 1.0 Length 36 B, Parameter Table Pointer 0x000030 Parsing JEDEC flash parameter table... 3-Byte only addressing. Status register is non-volatile and the standard does not allow vendors to tell us whether EWSR/WREN is needed for status … [View More]

1 0

Code Reviews guidelines discussion
by Anastasia Klimchuk 22 Apr '22

22 Apr '22

Hello People, Few weeks ago at the flashrom meeting we had a discussion about code reviews and the situations that sometimes happen. We decided that it would be good to have some guidelines… and then we decided that I will start writing something. I have been thinking about it since then. I even started writing something, but I wasn’t happy at all: it felt like composing a wall of text that no one would be reading. I decided to think about: what problems are we solving? And then it got much … [View More]

1 0

Unable to read “Boyamicro” SPI chip.
by Frankie LEE (HTX) 22 Apr '22

22 Apr '22

Hi, Unable to read “Boyamicro” SPI chip? Any workaround? Thanks. ============================================================================================== ack@ack:~$ flashrom -p ft2232_spi:type=2232H,port=B,divisor=4 -r firmware.bin flashrom v1.2 on Linux 5.13.0-40-generic (x86_64) flashrom is free software, get the source code at https://flashrom.org Using clock_gettime for delay loops (clk_id: 1, resolution: 1ns). Found Generic flash chip "unknown SPI chip (RDID)… [View More]

1 0

flashrom dev meeting notes 21.04.22
by Felix Singer 21 Apr '22

21 Apr '22

# 21th April 2022, 6:00 - 7:00 am UTC+0 Attendees: Thomas, David, Edward, Nico, Anastasia, Nikolai ## Decisions Summary * Anastasia will be doing next release. She has never done release before, and needs help from more experienced people. ## Agenda * [FelixS] Redmine is ready to use * If you wonder, new registrations require an unlock by an administrator. This is to prevent spamming and bots. Maybe we can work out something better in the future. * I added additional fields to … [View More]issues, so that important information is easily visible. For example affected hardware, affected OS, affected version. Please tell me if you find these useful or if you need other fields, trackers, .. * aklm: let’s start using, I will try to create an account and see how it goes * [FelixS] Created a proposal for closing GitHub issues and pull requests automatically * Closes newly created issues and pull requests and adds a comment using GitHub Actions hook. Old issues and pull requests stay untouched. * [flashrom patch](https://review.coreboot.org/c/flashrom/+/63671), based on GitHub Actions hook and [repo- lockdown](https://github.com/dessant/repo-lockdown). * My playground [https://github.com/felixsinger/awesome-repo](https://github.com/felixsinger… * [Nico] ChromeOS fork and attempted convergence * (in case witnesses of that time are present) What led to the fork? * [PatrickG] Velocity requirements: stuff needed to happen to make progress on the products using flashrom and it didn’t happen in time. Not much of a problem when it’s temporary but stuff just piled up over time. * Nico: lots of things went wrong. And it took around 8 months to converge * David: Fork happened when Carl-Daniel was the maintainer, and he was really busy. Tools were worse that time. * We had irreconcilable differences at that time * Edward: Stefan asked me to unfork flashrom. I had to work out how things were done, how to re-write parts of the system. It was just me, on my own, no other resources. I took upstream and tried to make chrome os work with upstream. Except for a few patches: mapping hack inside of sb600spi, I am trying to chase AMD to get documentation , so that I can improve it. * David: Timer stuff I was trying to deal locally. There were a bunch of things that had to be dealt locally. * David: It was uglier downstream than it should have been. Trying to minimize the number of hacks. There are things that can be tr * Edward: Where we are today: I was trying to have a team for flashrom, to spend dedicated time to work on flashrom. We had good results on that, but it takes time. Hard to allocate people. * (in case somebody knows) What is Google’s current strategy to avoid repeating mistakes of the past? * [PatrickG] Generally CrOS is willing to go upstream-first (as with coreboot) after divergence has been resolved. Depends on the upstream, so probably need to work on the questions raised by Edward et al. * [Nico] Upstream-first of a dominant vendor made it harder for others to work upstream on coreboot. It’s not what I had in mind, but this would be repeating a mistake, IMHO. * Nico: There were very good and very bad contributions during last years * Edward: how do you quantify that nothing is happening? * Nico : sb600 is bricking machines , but nothing is happening * Edward: maybe it is a visibility problem, also some things are really hard * Nico: maybe devs in google need to talk to each other? * Edward: original patch came from AMD, not from google * Nico: if it takes a long time to find a solution, maybe just revert? * Edward: we didn’t know it would take so long * David: board bringup. There is 3-4 months time before chromebooks get into users hands, and then bugs get filed. Bugs in the code which is 6 months old, but people are already reassigned to other tasks. * Edward: maybe have a parameter which turns on the behaviour, default is turned off. The problem exists, but it is not trivial. * Nico: The problem with the “upstream-first” idea: when something is not trivial, you push it upstream first, and then try to figure out how to fix it. * Edward: that was exceptional situation, it is better now * Nico: most of the issues in the list are from google. When someone looks at the code, it is clear that code it is not understood. * Edward: we opened issues about non-documented programmer, and added documentation * Nico: one of the drivers is mixing all layers of flashrom. * Nikolai: what are concerns with the code? We will try to remove them. * Edward: The datasheet was in mandarin, and lots of questions were discussed in mandarin with the vendor. Vendors were not giving documentation, despite us asking them many times. * Nico: Who is doing reviews for google? We need reviewers who know all layers of flashrom. * David: Can we simplify the process of adding a new chipset? So that it would be easier to contribute without introducing bugs? * Thomas: It would be nice to have more structure in the code repository. It is hard to understand where the code needs to go. Having a specific place for every component is a good idea * David : flash.h and flashrom.c are dumping grounds * Anastasia: Nico do you think it is possible so that we all work together normally and peacefully? If we resolve issues from the past, and also write guidelines so that new issues won’t appear and development goes normally? >> Yes it is possible. * Let's make a release! * Nico: I spent a lot of time on this already, I can’t spend even more time. Anyone else? * Nikolai: What if the vendor does not give documentation, do you think it’s better to have a driver which is not documented, or no driver at all? * David: for early chromebooks , drivers were not documented. That’s why an option was called “I want brick” * Edward: some of the changes in the chromium tree we are not sending upstream, because there is no documentation * Nikolai: Can we have a flag for situations like sb600spi ? Finer granularity than the whole driver. Feature flags? * Nico: other people work on the sb600spi code, affects everyone who works on the code. * Edward: something similar in kernel hardware quark framework. * Nico: this needs documentation, whatever is needed to understand the quark. Man pages are for the users, but there also should be documentation for developers. In what product is this chip? You can document what you know * [dhendrix] Is this the kind of finger pointing that Edward mentioned above? At this point, CrOS has proved more than happy to move upstream. The Github mirror is probably more of a concern. Perhaps upstream should consider how to get developers to choose upstream rather than the Github mirror or some other fork. * [Nico] Pointing at Google, actually. If one company forks and later wants their code upstream without proper review, something seems wrong. As if they never reflected on the problems. * [aklm] I am not sure we are on the same page on what are “mistakes of the past”. Everyone has their own idea on what the mistakes were. I can start from myself. From my observations, converging the code was considered a purely technical task, and so it was done as a technical task. However, converging the code means converging the people/the teams, converging very different workflows and work cultures. This does not seem to be thought through or ever discussed, and to me this is a mistake. * [quasisec] I am also unclear how CrOS convergence had really much of an impact on upstream considering >99% of the work was adjusting the CrOS fork to realign with upstream. Only an extremely small amount of alignment was in the form of upstream commits, e.g., missing raiden_debug and some unfortunate part of sb600spi. Is there a specific area that stands out? * [Nico] No idea if related to the original reasons for the fork, but that the convergence is stalling new releases for years stands out pretty much. * Anastasia: We currently have more GSOC project proposals than mentors (5 vs 3). If someone wants to be a mentor, it is not too late. Contact Anastasia or Felix. [View Less]

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

flashrom April 2022