search

linuxboot / heads

A minimal Linux that runs as a coreboot or LinuxBoot ROM payload to provide a secure, flexible boot environment for laptops, workstations and servers.
https://osresearch.net/
GNU General Public License v2.0
1.42k stars 187 forks source link

Bring dasharo+heads MSI boards from downstream Dasharo/heads to upstream #1753

Open tlaurion opened 3 months ago

tlaurion commented 3 months ago

NOTES:

DISCLAIMER: UNTESTED Sorry, not gonna cherry-pick commits from dahsharo/heads here, way too messy.

after merge, WILL superseed https://github.com/linuxboot/heads/pull/1489 abandonned effort.

TODO:

OLD ~DO NOT FLASH YET IF NO EXTERNAL PROGRAMMER. THIS VERSION BUMP FLASHROM SO FLASHING MASTER FROM THIS IS NEEDED~ Edit: fixed with #1755 commit cherry-picked.

tlaurion commented 3 months ago

@macpijan and others: not sure what this CONFIG_CBFS_VIA_FLASHROM in board configs is about as of now, most probably this PR totally invalid as of now, have not followed that white rabbit yet.

tlaurion commented 3 months ago

@macpijan and others: not sure what this CONFIG_CBFS_VIA_FLASHROM in board configs is about as of now, most probably this PR totally invalid as of now, have not followed that white rabbit yet.

1- @macpijan : commit log should be clear enough: Please validate this matches all required changes downstream please. 2- Also, can you tell why/what version of flashrom this is based on? 1.3.0+ with all WP + needed chipsets/spi chips needed? Is this expected to create regression on other boards support?

tlaurion commented 3 months ago

@macpijan I see this contains a coreboot version bump impacting nv41/ns50/MSI.

tlaurion commented 3 months ago

I think I picked all files based changes from https://github.com/linuxboot/heads/compare/master...Dasharo:heads:master visual queues, without rebasing nor cherry-picking, simply applying changes directly in files

Not gentle reminder, but requirement: changes should be proposed upstream, not forced like that or for upstream to pick up, even less when security vuln lingering

mkopec commented 3 months ago

2- Also, can you tell why/what version of flashrom this is based on?

The flashrom fork is based from this version: https://github.com/flashrom/flashrom/commit/053d319141db5954ab1ad34cb9db61983d238134

with added:

tlaurion commented 3 months ago

@macpijan updated op with commit logs sourced info, need comment on those changes and insights on impacts for other boards, most specifically:

tlaurion commented 3 months ago

@JonathonHall-Purism would need your eyes on this too, including regression testing for your boards since flashrom version bump

tlaurion commented 3 months ago

053d319

So flashrom 1.3.0-rc1 + patches, dating from upstream commit of ~2 years ago (cc @mkopec)

commit 053d319141db5954ab1ad34cb9db61983d238134 Author: Edward O'Callaghan quasisec@google.com Date: Fri Aug 12 21:27:41 2022 +1000

@JonathonHall-Purism: is this a problem? Seems like we could hit regressions here... Heads master was more recent.

@mkopec patches were not merged upstream in 1.4.0 now released? @macpijan ?

tlaurion commented 3 months ago

053d319

So flashrom 1.3.0-rc1 + patches, dating from upstream commit of ~2 years ago (cc @mkopec)

commit 053d319141db5954ab1ad34cb9db61983d238134 Author: Edward O'Callaghan quasisec@google.com Date: Fri Aug 12 21:27:41 2022 +1000

@JonathonHall-Purism: is this a problem? Seems like we could hit regressions here... Heads master was more recent.

@mkopec patches were not merged upstream in 1.4.0 now released? @macpijan ?

1.4.0 was released 2 weeks ago https://github.com/flashrom/flashrom/releases/tag/v1.4.0

@mkopec @macpijan is everything you need under 1.4.0? See https://github.com/flashrom/flashrom/compare/053d319...eace095

macpijan commented 3 months ago

@mkopec @macpijan is everything you need under 1.4.0?

I think the https://github.com/Dasharo/flashrom/commit/24b8fcfccef31fbb95bc1dd308180f57d5cdb64c will be missing. I cannot see it being submitted into upstream, can you confirm @miczyg1 ?

miczyg1 commented 3 months ago

@mkopec @macpijan is everything you need under 1.4.0?

I think the Dasharo/flashrom@24b8fcf will be missing. I cannot see it being submitted into upstream, can you confirm @miczyg1 ?

I think it is not in upstream flashrom (I believe we didn't even send a patch)

tlaurion commented 3 months ago

@mkopec @macpijan is everything you need under 1.4.0?

I think the Dasharo/flashrom@24b8fcf will be missing. I cannot see it being submitted into upstream, can you confirm @miczyg1 ?

I think it is not in upstream flashrom (I believe we didn't even send a patch)

@miczyg1 thats problematic. Bumped to 1.4.0 at 445d70f will try to put your unupstreamed patch into a Head patch. There is no way Head can regress to 2 years ago flashrom version since other stakeholders need flashrom too.

macpijan commented 3 months ago

@tlaurion Updated patch: https://github.com/Dasharo/flashrom/commit/7db77d39ae951cfd36765a91f6537377be34dad2

https://review.coreboot.org/c/flashrom/+/83854

tlaurion commented 3 months ago

@tlaurion Updated patch: Dasharo/flashrom@7db77d3

https://review.coreboot.org/c/flashrom/+/83854

EDITED: @macpijan fails to build see https://github.com/linuxboot/heads/pull/1753/commits/e8aaaabf43acd9aaba42bed317c82e86d647ddca

EDITED2: @miczyg1 patch won't build see above

macpijan commented 3 months ago

Another rev (the same link: https://review.coreboot.org/c/flashrom/+/83854) builds, tested to probe the chip on MSI.

tlaurion commented 3 months ago

@macpijan please review ab74127

tlaurion commented 3 months ago

@macpijan please review ab74127

Reviewed, could not spot any obvious problems in the configs ordering.

DDR5 board names have changed. @macpijan that's ok?

tlaurion commented 3 months ago

Regression testing with ab74127 built roms at https://app.circleci.com/pipelines/github/tlaurion/heads/2740/workflows/d9fabdea-65e1-4fa4-81a0-94a56a8a6aee for:

Needs testers to add to #692 otherwise these boards won't stay long as tested and will move to untested and unmaintained see https://github.com/linuxboot/heads/tree/master/unmaintained_boards:

Board testers need to be added under https://github.com/linuxboot/heads/blob/master/BOARD_TESTERS.md PRIOR OF MERGE OF THIS PR

tlaurion commented 3 months ago

Seems like https://github.com/Dasharo/flashrom/commit/6b2061bc0699202f81aeb782f301f1bba9f8a826 is missing from flashrom master, breaking Heads progress output hack since --progress still missing from flashrom....

Flashrom progress stuck at 0% when flashing internally to another rom from this PR, behavior exactly as under https://github.com/Dasharo/flashrom/pull/11#issuecomment-1409142374

Putting PR as draft again...

tlaurion commented 3 months ago

This is not enough: internal flashing to another ROM from this PR + current added patches still spins fast at 0% without percentage change when flashing overnight....

Seems like flashrom changed something again in their debug output (Heads takes verbose debug output and parses it to produce missing upstream progress bar output) Heads needs to patch again deeper???

https://github.com/user-attachments/assets/8aaf3e3a-afb9-4166-8a3d-54816e098815

WhatsUp flashrom.... It's expected to flash 32mb flash without progress output in 2024?!?!

tlaurion commented 3 months ago

Ok, so following rabbit hole where upstream --progress still unfixed for years https://ticket.coreboot.org/issues/390

To be edited with logs and maybe a fix....

Ha, maybe bad patch on my side, output diff (same HSFC: redundant and unneeded lines flooding output?) output_diff.txt

This is output of flashrom -p internal -w rom.rom -V > debug.log output from 1.3.0 on master vs 1.4.0 with patch supposed to fix exactly this from https://github.com/linuxboot/heads/pull/1753/commits/664df3b69189e6e60e4508a381d632624c731b5f

tlaurion commented 3 months ago

Don't get it, wiping build caches, building clean on CircleCI at https://app.circleci.com/pipelines/github/tlaurion/heads?branch=bring_downstream_Dasharo-Heads_msi_to_upstream with https://app.circleci.com/pipelines/github/tlaurion/heads?branch=bring_downstream_Dasharo-Heads_msi_to_upstream which fails because of mirror issues as well. Rebuilding builds from failed step until successfull.... Not a good day for testing.

Note that this issue not resolved nor patches added upstream, there is absolutely no reason to have more then 33k lines in debug repeating themselves, flashrom upstream!

(same HSFC: redundant and unneeded lines flooding output?) output_diff.txt

https://github.com/Dasharo/flashrom/pull/11#issuecomment-2282779936

Asked why upstream --progress still unfixed https://matrix.to/#/!EhaGFZyYcbyhdSgStq:matrix.org/$VmOqrKd4o-T2fHu2DAlP0WG67CyPO_H-o_A0PDvRWs4?via=matrix.org&via=sibnsk.net&via=nope.chat

tlaurion commented 3 months ago

Nope. Rebuilding clean (no cache whatsoever) from CI did a reproducible rom image for 230-maximized.

I will need help with this regression, will not have time to invest into this in foreseeable future @macpijan @JonathonHall-Purism and all other stakeholders benefiting from this with newer platforms needing flashrom version bumps while changes not upstreamed upstream requiring free maintenance downstream. Hacks not sustainable.

TLDR: flashrom 1.4 + patches in this PR breaks flashrom internal Heads progress output because flashrom still don't have --progress output which we would love to see landing since it makes no sense to flash in total blindness for more then 2 years now. Not a Heads problem, but an ecosystem problem, and I just stop personally trying to fix for free everyone's problem where collaboration upstream from people having a voice there would fix it once and for all and get rid of what was supposed to be a short time mitigation while things fixed upstream

Work can be picked up from https://github.com/Dasharo/flashrom/pull/11#issuecomment-2282779936 Past discussion with you @macpijan https://github.com/Dasharo/flashrom/pull/11#issuecomment-1409142374 Behavior observable from video at https://github.com/linuxboot/heads/pull/1753#issuecomment-2282761292 Workaround code to generate progress absent of flashrom from flashrom verbose output parsing under https://github.com/linuxboot/heads/blob/d9e5087caa06ab2ba3464b09c905ad7ef89b710b/initrd/bin/flash.sh#L151-L154

tlaurion commented 3 months ago

Coreboot channel attempted discussion waiting for answer https://matrix.to/#/!EhaGFZyYcbyhdSgStq:matrix.org/$9003TsHZPIZZIoTt7Z2cs6rJrciRFng0kALZThTEuJk?via=matrix.org&via=sibnsk.net&via=paranoia.wf

tlaurion commented 3 months ago

Dasharo general discussion downstram since we need a workaround if upstream don't fix it https://matrix.to/#/!rsKWMJGPMsyPTTjXuh:matrix.org/$N8PJecLPGi_tWckTmgBaeP0y5FFxdKNcPInqayUHmoo?via=matrix.org&via=nitro.chat&via=fedora.im

CC @macpijan

tlaurion commented 2 months ago

Cherry-picking #1755 commit

tlaurion commented 2 months ago

With #1755 cherry-picked: all is good and problem deflected upstream where it should be resolved: https://ticket.coreboot.org/issues/390

Decided: not Heads downstream problem to workaround infinitely, where solution for all flashrom users is to have flashrom --progress working, and default, for erase/write functions (missing, where no --progress is under 1.40 release still)

tlaurion commented 2 months ago

There is an important regression from this flashrom version bump and loss of in-house progress output.

Flashrom UX is regressing :/

So on master with flashrom 1.3 without in-house progress output of flashrom:

Under this PR flashrom 1.4 without in-house progress output of flashrom:

tlaurion commented 2 months ago

Question asked under coreboot matrix channel https://matrix.to/#/!EhaGFZyYcbyhdSgStq:matrix.org/$9K9t1rkGi2wPL6iHc3Hde2EhLkVbEN3N07Y1dFU3lek?via=matrix.org&via=sibnsk.net&via=nope.chat

tlaurion commented 2 months ago

Question asked under Dasharo General matrix room: https://matrix.to/#/!rsKWMJGPMsyPTTjXuh:matrix.org/$1l2yMDyYBoGeU-1KNAI2e-QLMpfGgav4e_pBqg8J_qQ?via=matrix.org&via=nitro.chat&via=fedora.im

tlaurion commented 2 months ago

Went ahead and worked around flashrom UX change merging https://github.com/linuxboot/heads/pull/1756 and merging master into this PR.

tlaurion commented 2 months ago

Verified https://review.coreboot.org/c/flashrom/+/83854

git fetch https://review.coreboot.org/flashrom refs/changes/54/83854/3 && git format-patch -1 --stdout FETCH_HEAD > patches/flashrom-eace095b15eb034e42d97202cad70ce979d8ca38/0001-Add_RaptorPoint_PCH_support.patch

Produces no change. Ready for testing when roms are produced. @macpijan can you test on MSI please CircleCI produced roms for MSI variants? Thanks.

tlaurion commented 2 months ago

Tested per flashing internally twice CircleCI pipeline's boards's artifact's zip/tar.gz(talos) packed rom images from https://app.circleci.com/pipelines/github/tlaurion/heads/2770/workflows/d4aa1ab1-2feb-45d9-937d-8a06fb4579be :

tlaurion commented 2 months ago

Bricked my nv41...

tlaurion commented 2 months ago

Seems like flashrom 1.4 is missing some patches for nv41.

@macpijan ? See picture at https://github.com/linuxboot/heads/pull/1753#issuecomment-2307524080

SergiiDmytruk commented 2 months ago

I made some WIP patch for progress (here):

Demonstration

Erasing is so fast probably because whole chip erase command is used, in this case the progress can only go from 0% to 100%. ![flash-progress-2](https://github.com/user-attachments/assets/dab7a8f8-ef68-46c6-9d94-57de1f4dc4e4)

Not really planning to finish it, just took a look at the code, had a laugh and had to give it a try. Then decided to not throw it away.

tlaurion commented 2 months ago

@SergiiDmytruk the more pressing need is that using flashrom 1.4 fails to flash anything else internally as can be seen from https://github.com/linuxboot/heads/pull/1753#issuecomment-2307524080 :

  • [ ] nv41 -> @tlaurion FAILED
    • PXL_20240823_175752907.MP.jpg
    • Putting back in draft....

I pinged coreboot channel with your PoC at https://matrix.to/#/!EhaGFZyYcbyhdSgStq:matrix.org/$sCxMz4aeYlbiAHh_cvGajmIEw-XISnBd4UFilwcibZQ?via=matrix.org&via=sibnsk.net&via=nope.chat this is awesome progress already, but that would not fix 1.4 not being usable for this PR without more patches added alongside patches/flashrom-eace095b15eb034e42d97202cad70ce979d8ca38/0001-Add_RaptorPoint_PCH_support.patch :(

tlaurion commented 2 months ago

I made some WIP patch for progress (here):

Demonstration Erasing is so fast probably because whole chip erase command is used, in this case the progress can only go from 0% to 100%.

flash-progress-2 flash-progress-2

Not really planning to finish it, just took a look at the code, had a laugh and had to give it a try. Then decided to not throw it away.

@SergiiDmytruk this should be an input for https://ticket.coreboot.org/issues/390 progressing to being fixed with collaboration, hopefully. Thanks for caring for this, this is already something, if known from the ticket itself aimed to be attempted to be fixed.

Curious: Why did you laugh at the code?

tlaurion commented 2 months ago

Seems like flashrom 1.4 is missing some patches for nv41.

@macpijan ? See picture at #1753 (comment)

TLDR: @SergiiDmytruk this is the blocker for this PR.

SergiiDmytruk commented 2 months ago

Curious: Why did you laugh at the code?

Commit comment on gerrit explains it: size_t end_address = len - start; makes zero sense, len can even be smaller than start.

Seems like flashrom 1.4 is missing some patches for nv41.

Don't know about NovaCustom. Might be a bug of flashrom which you just happen to hit on this hardware. You can try flipping https://github.com/flashrom/flashrom/blob/eace095b15eb034e42d97202cad70ce979d8ca38/flashrom.c#L39 (e.g., with sed for a build) and see whether the bug goes away.

macpijan commented 2 months ago

Seems like flashrom 1.4 is missing some patches for nv41.

I am not aware of any extra patches needed for ADL-P.

We have not been carrying any for it, on the older flashrom version.

On your screen, the chipset gets detected just fine.

One important difference with newer flashrom is that on Intel chipsets it seems to detect the exect flash chip, not just the opaque flash chip as it used to before. Maybe there is some problem here, or with support for this particular chip.

tlaurion commented 2 months ago

@macpijan @SergiiDmytruk : not only legacy is getting deprecated but code was deleted recently. Houston, we have a problem https://review.coreboot.org/c/flashrom/+/83852/2

SergiiDmytruk commented 2 months ago

not only legacy is getting deprecated but code was deleted recently.

That happened after v1.4 release and thus doesn't affect this PR which points at https://github.com/flashrom/flashrom/commit/eace095b15eb034e42d97202cad70ce979d8ca38. Maybe the new code works fine, I don't know much about it.

tlaurion commented 2 months ago

not only legacy is getting deprecated but code was deleted recently.

That happened after v1.4 release and thus doesn't affect this PR which points at flashrom/flashrom@eace095. Maybe the new code works fine, I don't know much about it.

@SergiiDmytruk My point is that even if I switched to legacy, this codepath is now deprecated so patch downstream would not be picked up in next release. Definitely a bug and not a feature. What's the way forward here to have this fixed in master?

You can test this PR on one of your nv41, on my side my wson probe will die and I cannot afford a brick I cannot recover at this point.

SergiiDmytruk commented 2 months ago

Maybe the issue is somewhere else, it was just a guess. If there is a bug in that code, it will likely depend more on flash chip contents/operation rather than on hardware, because that code doesn't deal directly with hardware.

tlaurion commented 2 months ago

@i-c-o-n: is flashprog

If so, I will invest time switching from flashrom to flashprog. Flashrom is a real mess, ecosystem fragmented: sigh.

i-c-o-n commented 2 months ago

@i-c-o-n: is flashprog

  • supporting flashrom partial region WP

WP is a huge topic, what do you need specifically?

What works (since ~2018):

Support to configure protections of the flash chip itself is WIP, with the CLI design needing some opinion. I don't have a use-case right now, so I didn't want to merge it when no potential user had a look at it. Support to perform such configuration with the internal programmer on Intel systems is delicate and not worked on yet.

  • Ivy, sandy, haswell, alder lake, raptor lake

yes, yes, yes, yes, and yes but Raptor Lake is untested (I don't expect any issues as the hardware should behave the same)

Still opaque with flashprog. I'm not aware of any regression.

Yes, quite similar, plus a moving bar.

If so, I will invest time switching from flashrom to flashprog. Flashrom is a real mess, ecosystem fragmented: sigh.

Thanks. I know the situation bugs everybody. If I hadn't seen similar issues two years ago already, I never would have agreed to the fork.

tlaurion commented 2 months ago

@i-c-o-n: is flashprog

  • supporting flashrom partial region WP

WP is a huge topic, what do you need specifically?

Raw braindump.

As of today, only thing used in the wild (known to be used) is for d16:

For other platforms, some are adventuring, but soldering/unsoldering WP to GND is making this work for highly paranoid people:

WP work funded by NlNet which you can see deliverables under https://github.com/linuxboot/heads/issues/1741 and follow white rabbit there.

What works (since ~2018):

  • Writing specific regions when others are write-protected. The user has to specify the layout (e.g. --layout/--ifd/--fmap).
  • Reading/writing specific regions when others are read-protected. Requires a layout as above, writes additionally need --noverify-all.

Support to configure protections of the flash chip itself is WIP, with the CLI design needing some opinion. I don't have a use-case right now, so I didn't want to merge it when no potential user had a look at it. Support to perform such configuration with the internal programmer on Intel systems is delicate and not worked on yet.

  • Ivy, sandy, haswell, alder lake, raptor lake

yes, yes, yes, yes, and yes but Raptor Lake is untested (I don't expect any issues as the hardware should behave the same)

Still opaque with flashprog. I'm not aware of any regression.

Yes, quite similar, plus a moving bar.

If so, I will invest time switching from flashrom to flashprog. Flashrom is a real mess, ecosystem fragmented: sigh.

Thanks. I know the situation bugs everybody. If I hadn't seen similar issues two years ago already, I never would have agreed to the fork.

Will look into this after your feedback on WP above @i-c-o-n .

Thanks for doing what you do, much much appreciated, the ecosystem is thanking you in silence, I do it here in all gratitude.

i-c-o-n commented 2 months ago

WP is a huge topic, what do you need specifically?

Raw braindump.

As of today, only thing used in the wild (known to be used) is for d16:

...

That's the topic that would need the aforementioned CLI update. Patches are written for flashprog, but only been rebased in the past 18 months. I'll give them a look and a little testing later.

The flashchip database may need some updates for specific cases. But those can usually be written and reviewed within hours.

Fortunately the D16 is AMD, which should just work the same as external programmers. But extending this to Intel is a bummer, hard to make brick resilient because one is partially flying blindly (unless using an external programmer).

coreboot has limited support for this too, btw. Around config BOOTMEDIA_LOCK_CHIP.

Support to configure protections of the flash chip itself is WIP, with the CLI design needing some opinion. I don't have a use-case right now, so I didn't want to merge it when no potential user had a look at it. Support to perform such configuration with the internal programmer on Intel systems is delicate and not worked on yet.

i-c-o-n commented 2 months ago

WP is a huge topic, what do you need specifically?

Raw braindump. As of today, only thing used in the wild (known to be used) is for d16: ...

That's the topic that would need the aforementioned CLI update. Patches are written for flashprog, but only been rebased in the past 18 months. I'll give them a look and a little testing later.

Still working fine :smile: Added a few bits for the chips I had at hand. More testing welcome!