Switch from flashrom to flashprog #1769
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| modules-$(CONFIG_FLASHPROG) += flashprog | ||
|
|
||
| flashprog_depends := pciutils $(musl_dep) | ||
|
|
||
| flashprog_version := 9dc6d843b0678001c9baf0e8602ecb25b16329d2 | ||
| flashprog_dir := flashprog-$(flashprog_version) | ||
| flashprog_tar := $(flashprog_dir).tar.gz | ||
| flashprog_url := https://github.com/SourceArcade/flashprog/archive/$(flashprog_version).tar.gz | ||
| flashprog_hash := fa4ddf3b60314994a37e4599122ae4c7f42135c13c782e580bc580d715cfa2fc | ||
|
|
||
| # Default options for flashprog | ||
| flashprog_cfg := \ | ||
| WARNERROR=no \ | ||
| CONFIG_NOTHING=yes \ | ||
| CONFIG_INTERNAL=yes \ | ||
| CONFIG_INTERNAL_X86=yes \ | ||
|
|
||
| ifeq "$(CONFIG_TARGET_ARCH)" "ppc64" | ||
| flashprog_cfg := \ | ||
| WARNERROR=no \ | ||
| CONFIG_NOTHING=yes \ | ||
| CONFIG_LINUX_MTD=yes | ||
| endif | ||
|
|
||
| #Only enable AST1100 if requested per board configs | ||
| ifeq "$(CONFIG_FLASHPROG_AST1100)" "y" | ||
|
There was a problem hiding this comment. Patch missing. Even i currently activated under server board and passed at make, doesn't o anything without https://github.com/linuxboot/heads/blob/master/patches/flashrom-b1f858f65b2abd276542650d8cb9e382da258967/0100-enable-kgpe-d16.patch missing There was a problem hiding this comment. I can have a look at it. AFAIR, nobody made a real effort to upstream this. If I rebase this, we'd still need somebody to test it thoroughly. We are quite pedantic when it comes to internal flashing. There was a problem hiding this comment. @i-c-o-n it is a problem of upstreaming indeed, keeping track of patches downstream never proved to do any good. If not under flashprog/flashrom: this means end users need to flash bmc externally which is not a desirable thing. Also as you may know, that bmc port is old but still in charge of best fan control out there for the d16. Having heads flash through https://github.com/linuxboot/heads/pull/1769/files#diff-08a8a9080a4ca4377e0de4dbef6bd94c84209343b4e12a8982352a1c1b09b71b would be useful. That would be ideal, but not a blocker for this PR. There was a problem hiding this comment. @i-c-o-n do you see anything else that should be improved in the flashprog makefile for size/functionalities inclusion? There was a problem hiding this comment. Do you see anything that should be added in board config for flash options that should be passed? Something that would make speed of internal flashing faster? Also not a requirement for this PR, but a nice thing to track in separate issue for improvements, let me know. There was a problem hiding this comment.
No, the current selection is already the minimum AFAICT. Anything else would need some hacking on the code. One low hanging fruit: For targets that use Intel hwseq or linux_mtd, you could drop everything else from
|
||
| flashprog_cfg += CONFIG_AST1100=yes | ||
| endif | ||
|
|
||
| flashprog_target := \ | ||
| $(MAKE_JOBS) \ | ||
| CFLAGS="-Os -I$(INSTALL)/include/pci" \ | ||
| DESTDIR="$(INSTALL)" \ | ||
| INSTALL="$(INSTALL)" \ | ||
| LDFLAGS="-L$(INSTALL)/lib" \ | ||
| PREFIX="$(INSTALL)" \ | ||
| $(CROSS_TOOLS) \ | ||
| $(flashprog_cfg) \ | ||
| flashprog | ||
|
|
||
| flashprog_output := \ | ||
| flashprog | ||
|
|
||
| flashprog_libraries := \ | ||
|
|
||
| flashprog_configure := | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@i-c-o-n still good commit? would wp_cli branch need update prior of merging? next iteration?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's sitting ducks for review. I don't expect bigger changes.
@SergiiDmytruk know any users of the block protection that could give some feedback on the CLI?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe users who commented in flashrom/flashrom#185? My impression on WP is that people are interested in it for a relatively short period of time until they realize its practical limitations @i-c-o-n
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haha, yeah, it's not just a give-me-security switch. Modern chips don't even have a /WP pin (because they prefer quad-i/o). Biggest bummer though is the lacking support to read/write all registers on Intel platforms (I have one more idea to try but little hope), otherwise one could use the protect-until-powercycle feature.
@ln2max, @jtf7 looking for user test and feedback on the WP CLI available through this PR, or this branch. It's not too different from flashrom's, e.g.
wp --statusinstead of--wp-status. And it supports only one command at a time to be sure about the execution order.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@i-c-o-n are you talking about chipset locking (PR0) as per #1373 (comment) OP's todo pointing to @root-hardenedvault 's x11 downstream (non-upstreamed) work for Haswell+ platforms https://github.com/hardenedvault/vaultboot/blob/master/patches/coreboot/0001-x11.patch ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, there's another feature of the flash chip itself. Most common for SPI flashes is that you can configure which blocks to protect and then there are four states:
For 2. we need the pin and support or modification of the mainboard. 3. and 4. often require to write a bit in a secondary register. Problem is so far nobody knows how to write that register with Intel's
hwseq, which is the only option since Skylake. I don't think Intel implemented that. They just want you to use their protection mechanisms like PR0; those are, however, very inconvenient when Intel FSP is involved.Something like that hardenedvault patch wouldn't work on (at least some) newer systems because FSP would lock PR0 too early, unconditionally. Security with Intel is never easy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(tlaurion edit: tagging @i-c-o-n) :
I thought chips supporting this feature are fairly uncommon. I remember seeing notes in a bunch of datasheets to the effect of "this feature is available per request" (maybe all from a single vendor).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm, my recollection may be biased by our own chip selection (we used this feature on an AMD based system almost 15 years ago). I thought it's very common for Winbond; and GD and XMC for instance seem to copy everything from them judging from datasheets. Macronix seems to be an exception. This "available per request" does ring a bell, but I usually see this regarding 4., the permanent locking.
(tlaurion edit : please tag people to trigger a notification to external people directly. Here @SergiiDmytruk)