Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WiP: Switch master flashrom to dasharo/flashrom #1251

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tlaurion
Copy link
Collaborator

@tlaurion tlaurion commented Dec 1, 2022

Dasharo/flashrom includes many changes that are not included inside of flashrom/flashrom project:

We couldn't switch to dasharo/flashrom prior of this patch to not cause regression on KGPE-D16 platform.
Note that as of today, having CircleCI build the kgpe-d16 boards fail on top of debian-11 docker image because of a race condition inside of the builds. Also note that KGPE-d16 and librem server, based on coreboot 4.11, were removed from CircleCI for those reasons a while back.


Transient build test of flashrom against version reported in commit to test Dasharo/flashrom#11 until successful

To test prior of merging on Heads side:

@tlaurion tlaurion marked this pull request as draft December 1, 2022 16:09
@tlaurion tlaurion changed the title WiP: Switch mster flashrom to dasharo/flashrom WiP: Switch master flashrom to dasharo/flashrom Dec 1, 2022
@tlaurion tlaurion force-pushed the dasharo_flashrom_test branch 2 times, most recently from 25c5986 to 8e28a32 Compare December 1, 2022 21:34
@tlaurion tlaurion force-pushed the dasharo_flashrom_test branch 2 times, most recently from 48de52f to 87dcfcd Compare January 16, 2023 22:29
@tlaurion
Copy link
Collaborator Author

tlaurion commented Jan 16, 2023

@tlaurion
Copy link
Collaborator Author

Reported outcome of switching flashrom 1.2 to dasharo/flashrom from latest commit at Dasharo/flashrom#11 (comment) (cross-referencing)

@tlaurion tlaurion force-pushed the dasharo_flashrom_test branch 2 times, most recently from 9243702 to ac7ebfb Compare January 22, 2023 19:38
@tlaurion tlaurion force-pushed the dasharo_flashrom_test branch 2 times, most recently from d01a3d5 to d0aab9a Compare January 23, 2023 16:22
@tlaurion
Copy link
Collaborator Author

tlaurion commented Jan 23, 2023

Now compiles correctly for x86 (NOTHING + Internal + dummy + ast1100 for kgpe-d16 support) but fails for ppc64 (Talos II with NOTHING + mtd + dummy + AST2400)

Reported at Dasharo/flashrom#11 (comment)

d0aab9a adds flashrom dependency for libusb to be built firts, which even if uneeded, flashrom tests against (is not a reason for current build failing though, but added for good measure)

@tlaurion
Copy link
Collaborator Author

Newer flashrom don't accept compile option as a make argument, but requires those configuration options to be passed as environment variables preceding the make call.

Consequently, prior builds were compiling the whole flashrom binary, not respecting CONFIG_NOTHING and then building only modules/flashrom arch specific statements (x86 builds for internal, dummy and ast1100, while ppc64 builds now only for the MTD programmer) needed to upgrade internally.

@tlaurion
Copy link
Collaborator Author

Tested on x230-maximized:

internal programmer is not in?! nicintel is in? Will rebuild from clean build...
signal-2023-01-30-101737

@tlaurion tlaurion force-pushed the dasharo_flashrom_test branch 2 times, most recently from 40c653f to 135aeee Compare January 30, 2023 16:30
@tlaurion
Copy link
Collaborator Author

tlaurion commented Jan 30, 2023

Resetted CircleCI project's setting env variable's CACHE_VERSION to 2023-01-16 to reuse cache. Restarting build

tlaurion added a commit to tlaurion/heads that referenced this pull request Feb 19, 2023
…ix-sh_argument_expected-yubikey-oem-factory-reset_dasharo-flashrom

Daily driver test fo x230-hotp-maximized on coreboot 4.19, with debug, yubikey test regression for oem-factory-reset, optimized for space (03-O2->Os) and fix for sh: argument expected, with local CONFIG_DEBUG_OUTPUT enabled and fused in ROM.
    Includes linuxboot#1317, linuxboot#1121, linuxboot#1312, linuxboot#1305, linuxboot#1251 for test on daily driver
tlaurion referenced this pull request in tlaurion/heads Feb 19, 2023
…ix-sh_argument_expected-yubikey-oem-factory-reset_dasharo-flashrom

Daily driver test fo x230-hotp-maximized on coreboot 4.19, with debug, yubikey test regression for oem-factory-reset, optimized for space (03-O2->Os) and fix for sh: argument expected, with local CONFIG_DEBUG_OUTPUT enabled and fused in ROM.
    Includes osresearch#1317, osresearch#1121, osresearch#1312, osresearch#1305, osresearch#1251 for test on daily driver
@tlaurion
Copy link
Collaborator Author

Using x230-hotp-maximized from CircleCI builds as of now. Nothing to report as of now as regression.

@tlaurion
Copy link
Collaborator Author

tlaurion commented Feb 22, 2023

Past patches have been upstreamed into dasharo/flashrom Dasharo/flashrom@6b2061b

changing commit and rebasing on master

@tlaurion
Copy link
Collaborator Author

tlaurion commented Feb 22, 2023

@tlaurion
Copy link
Collaborator Author

tlaurion commented Feb 22, 2023

Local build before passing to -Os


"/home/user/heads/build/x86/coreboot-4.13/x230-maximized/cbfstool" "/home/user/heads/build/x86/coreboot-4.13/x230-maximized/coreboot.rom" print
FMAP REGION: COREBOOT
Name                           Offset     Type           Size   Comp
cbfs master header             0x0        cbfs header        32 none
fallback/romstage              0x80       stage           85100 none
cpu_microcode_blob.bin         0x14d80    microcode       26624 none
fallback/ramstage              0x1b600    stage           97670 none
config                         0x333c0    raw               839 none
revision                       0x33740    raw               691 none
fallback/dsdt.aml              0x33a40    raw             14615 none
vbt.bin                        0x373c0    raw              1433 LZMA (4281 decompressed)
cmos_layout.bin                0x379c0    cmos_layout      1884 none
fallback/postcar               0x38180    stage           25816 none
fallback/payload               0x3e6c0    simple elf    7314375 none
(empty)                        0x7382c0   null          4750040 none
bootblock                      0xbbfdc0   bootblock       65536 none
2023-02-22 12:13:52-05:00 INSTALL   build/x86/coreboot-4.13/x230-maximized/coreboot.rom => build/x86/x230-maximized/heads-x230-maximized-v0.2.0-1378-g52035b8.rom

@tlaurion
Copy link
Collaborator Author

tlaurion commented Feb 22, 2023

Local build after passing -Os

"/home/user/heads/build/x86/coreboot-4.13/x230-maximized/cbfstool" "/home/user/heads/build/x86/coreboot-4.13/x230-maximized/coreboot.rom" print
FMAP REGION: COREBOOT
Name                           Offset     Type           Size   Comp
cbfs master header             0x0        cbfs header        32 none
fallback/romstage              0x80       stage           85100 none
cpu_microcode_blob.bin         0x14d80    microcode       26624 none
fallback/ramstage              0x1b600    stage           97706 none
config                         0x33400    raw               845 none
revision                       0x337c0    raw               697 none
fallback/dsdt.aml              0x33ac0    raw             14615 none
vbt.bin                        0x37440    raw              1433 LZMA (4281 decompressed)
cmos_layout.bin                0x37a40    cmos_layout      1884 none
fallback/postcar               0x38200    stage           25816 none
fallback/payload               0x3e740    simple elf    7314887 none
(empty)                        0x738540   null          4749400 none
bootblock                      0xbbfdc0   bootblock       65536 none

4750040 - 4749400=positive footprint (increase of 640 bytes?!?!)

@tlaurion
Copy link
Collaborator Author

tlaurion commented Feb 22, 2023

Using x230-hotp-maximized as a reference, just because this is board that is building from clean commit over CI (thanks to cache layers, since a module/* changed, we are reusing coreboot cache + musl-cross-cache to speed up the builds, without using package build cache as can be seen under https://app.circleci.com/pipelines/github/tlaurion/heads/1487/workflows/cd665149-b87a-49b1-9ecb-162772c2f27d/jobs/17348/parallel-runs/0/steps/0-108).

x230-maximized without -Os (Was -O2):
https://app.circleci.com/pipelines/github/tlaurion/heads/1487/workflows/cd665149-b87a-49b1-9ecb-162772c2f27d/jobs/17350/parallel-runs/0/steps/0-103 output

"/root/project/build/x86/coreboot-4.13/x230-hotp-maximized/cbfstool" "/root/project/build/x86/coreboot-4.13/x230-hotp-maximized/coreboot.rom" print
FMAP REGION: COREBOOT
Name                           Offset     Type           Size   Comp
cbfs master header             0x0        cbfs header        32 none
fallback/romstage              0x80       stage           85100 none
cpu_microcode_blob.bin         0x14d80    microcode       26624 none
fallback/ramstage              0x1b600    stage           97670 none
config                         0x333c0    raw               834 none
revision                       0x33740    raw               691 none
fallback/dsdt.aml              0x33a40    raw             14615 none
vbt.bin                        0x373c0    raw              1433 LZMA (4281 decompressed)
cmos_layout.bin                0x379c0    cmos_layout      1884 none
fallback/postcar               0x38180    stage           25816 none
fallback/payload               0x3e6c0    simple elf    7336903 none
(empty)                        0x73dac0   null          4727512 none
bootblock                      0xbbfdc0   bootblock       65536 none

x230-hotp-maximized with -Os passed https://app.circleci.com/pipelines/github/tlaurion/heads/1488/workflows/bba93be4-e40c-4673-b2ff-3f98cf593e37/jobs/17353/parallel-runs/0/steps/0-103

"/root/project/build/x86/coreboot-4.13/x230-hotp-maximized/cbfstool" "/root/project/build/x86/coreboot-4.13/x230-hotp-maximized/coreboot.rom" print
FMAP REGION: COREBOOT
Name                           Offset     Type           Size   Comp
cbfs master header             0x0        cbfs header        32 none
fallback/romstage              0x80       stage           85100 none
cpu_microcode_blob.bin         0x14d80    microcode       26624 none
fallback/ramstage              0x1b600    stage           97660 none
config                         0x333c0    raw               834 none
revision                       0x33740    raw               691 none
fallback/dsdt.aml              0x33a40    raw             14615 none
vbt.bin                        0x373c0    raw              1433 LZMA (4281 decompressed)
cmos_layout.bin                0x379c0    cmos_layout      1884 none
fallback/postcar               0x38180    stage           25816 none
fallback/payload               0x3e6c0    simple elf    7331783 none
(empty)                        0x73c6c0   null          4732632 none
bootblock                      0xbbfdc0   bootblock       65536 none

4732632-4727512=5120
Gain of 5210 bytes. Keeping it in.

@JonathonHall-Purism : regression on librems? (Internal programmer worked in past tests on x230 and mtd for Talos)

tlaurion added a commit to tlaurion/heads that referenced this pull request Mar 1, 2023
Adresses @easrentai suggestion to pass modules build optimization for space here: linuxboot#590 (comment)

- Uniformized module's $(CROSS_TOOLS) being passed as environment variable, prior of ./configure call
- Includes linuxboot#1251 (should be rebased on top of it merged)
@tlaurion
Copy link
Collaborator Author

tlaurion commented Mar 2, 2023

Increase of 12% (122kb) in size footprint as opposed to flashrom currently used under master....

@JonathonHall-Purism
Copy link
Collaborator

@tlaurion I tested this on Librem 14 and L1UM, functionality is fine. I do see the 127K size increase though. I took a look over the commit log and we gained a lot of commits (many upstream, some from dasharo), maybe we can bisect the size increase?

…11246 from work under  https://github.com/Dasharo/flashrom/tree/kgpe-patch-rebase

Pointing to Dasharo/flashrom#11 so that CircleCI shows success where work is happening

Changes:
- "WARNERROR=no" is a env variable interpreted at compilation now, not a configuration option anymore
- ~To work around heads pkg-config, newer flashrom needs to have LIBS_BASE overriden to detect proper libusb and libpci as installed under heads/install~ fixed upstream in previous commits
- INSTALL="$(INSTALL)" DESTDIR="$(INSTALL)" CFLAGS="-I$(INSTALL)/include/libusb-1.0 -I$(INSTALL)/include/pci" and LDFLAGS="-L$(INSTALL)/lib" needs to be passed as env variable  to build properly
- flashrom module now depends on libusb, since flashrom looks for pkg-config of installed libusb as prereq
- flashrom ppc64: remove ast2400 and dummy, leaving NOTHING+MTD only

NOTES:
- newer flashrom version seems to need to have environment variables defined prior of make call on console, not passing options at make call
- CONFIG_INTERNAL is not enough to have internal programmer anymore on x86. CONFIG_INTERNAL_X86 also needs to be requested.

Collaboration happened under Dasharo/flashrom#11
@tlaurion
Copy link
Collaborator Author

tlaurion commented Apr 13, 2023

rebased on master

@tlaurion
Copy link
Collaborator Author

tlaurion commented Apr 13, 2023

#1371 (comment) shows that current cost of passing to newer flashrom with ast1100 support and WP is 0.01mb

The main concern is always t520-maximized board https://app.circleci.com/pipelines/github/tlaurion/heads/1605/workflows/26753865-ac5a-4097-8e06-eccc85d748bf/jobs/20855?invite=true#step-103-888 which would now have 0.87mb empty after merge.

@rbreslow : another newer addition is the t440p-hotp-maximized, which after this merge (and mrc.bin requirement to fixate CBFS_SIZE to 8mb) would now have https://app.circleci.com/pipelines/github/tlaurion/heads/1605/workflows/26753865-ac5a-4097-8e06-eccc85d748bf/jobs/20936?invite=true#step-103-818 0.178mb left prior of failing building.... EDIT: there is another empty region of 0.67mb at https://app.circleci.com/pipelines/github/tlaurion/heads/1605/workflows/26753865-ac5a-4097-8e06-eccc85d748bf/jobs/20936?invite=true&invite=true#step-103-816, not sure how cbfs add will deal with file injection, but it is expected that no problem there.

@tlaurion
Copy link
Collaborator Author

tlaurion commented Apr 13, 2023

@JonathonHall-Purism In the light of prior comments, are we willing to merge this?

maybe we can bisect the size increase?

Well, we could. But we might find more space optimizing kernel options even more

@JonathonHall-Purism
Copy link
Collaborator

Yes, we can merge this. We discussed separately whether these changes will be upstreamed (dasharo is 244 commits behind right now), hopefully at least the WP bits will be. If we run into any issue down the line where we need something from upstream I'm willing to revisit then.

@tlaurion
Copy link
Collaborator Author

To be tested on top of #1381

@tlaurion
Copy link
Collaborator Author

tlaurion commented Jun 25, 2023

Wp support is now part of flashrom 1.3.
Flashrom 1.3 adds internal_x86 config option which seems in turn to be able to use MSR if enabled in kernel and could permit removal of genetic internal support, which compiles all chips and supported chipsets on flashrom side and where the kernel could handle it without code duplication?

Needs testing otherwise builds fails combined with gnupg 2.4 and 5.10.x on legacy platforms, reopening consideration to drop legacy board support.

As of today, ast1100 supoort, which dahsaro-flashrom is providing, doesn't seem to be used/tested by the community. It is aimed to support BMC spi flashing from heads and requires optional asmb4 chip which noooe seems to own beside myself, also leading to max fan speed since fan regulation on kgpe-d16 depends on the BMC doing that job, leading people to complain about kgpe-d16 electricity consumption, combined with CPU watt consumption which is higher combined to other newer platforms.


# Default options for flashrom
flashrom_cfg := \
WARNERROR=no \
CONFIG_NOTHING=yes \
CONFIG_INTERNAL=yes \
CONFIG_INTERNAL_X86=yes \
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try removing CONFIG_INTERNAL

@tlaurion
Copy link
Collaborator Author

Further testing under #1420

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants