heads
heads copied to clipboard
linuxboot
WiP: PR0 (SPI write prevention through chipset locking) for skylake and more recent
@miczyg1 pushed some changes in dasharo coreboot pork to enable SMM PR0: This cherry-pick this commit https://github.com/Dasharo/coreboot/commit/ff22122c229bbe2109de92ded773493428f7ece9
To test this PR for nv41 users (EC needs to be up to date otherwise problems might occur):
- Flash this PR rom
- go to recovery shell
- call lock_chip from command line
- call config-gui.sh
- change settings (Eg : enable debug)
- flash changes back to SPI, see failing (expected)
At term, this PR will bring coreboot skylake+ equivalent of <=haswell for SPI lock. @MrChromebox you might take a look at https://github.com/linuxboot/heads/issues/1659, which this PR will fix when working on all skylake+ based boards.
At term, this will fix https://github.com/linuxboot/heads/issues/1659
TODO:
- [x] nv41 functional, flashrom tested internally after calling lock_chip: unsuccessful (ok!)
- [ ] fix and merge #1814
- [ ] add patch to other forks
- [ ] or merge this PR while other forks pick it up?
- [ ] adapt lock_chip and verbiage if not just fro pre-skylake in codebase
- [ ] +1 review on coreboot when patchset tested and working for all skylake+
--
@JonathonHall-Purism patch doesn't apply cleanly on purism fork
@miczyg1 false alarm, it was again my external programmer's fault. Tigard doesn't work well but again I have a leg of wson8 probe broken.
This is good to go!
Question here is what do we want prior of merging @miczyg1 @JonathonHall-Purism.
All Skylake+ platforms implementing PR0?
Question here is what do we want prior of merging @miczyg1 @JonathonHall-Purism.
All Skylake+ platforms implementing PR0?
@macpijan @JonathonHall-Purism let me know if something else needs to be done prior of #1821 deadline. Could be in or not: I would prefer this to be in.
@tlaurion @miczyg1 Nice work here, I'm thrilled to see this. I hope we can get it to work :crossed_fingers:
What SoCs has this been tested on so far? I applied the coreboot patch to Purism's coreboot fork and built this for the Librem 14; the OS is still able to write to the firmware. The coreboot patch is implemented for cannonlake but I don't know if that was tested.
I did see Finalizing chipset Write Protection through SMI PR0 lockdown call, so the config is right and Heads did appear to try to lock flash. (Full disclosure, I booted manually with basic-autoboot.sh from the recovery shell, but this uses the same kexec-boot path and I did see that trace.)
The OS was still able to write to flash (in this case I changed the serial number using our coreboot utility: that reads flash, uses cbfstool to change the serial, then writes flash).
I haven't looked into it any further. What SoCs have you all tested? I'd be happy to help get this working for CNL and other SoCs but I probably will not be able to do it prior to the feature freeze.
Question here is what do we want prior of merging @miczyg1 @JonathonHall-Purism.
All Skylake+ platforms implementing PR0?
I'd suggest:
- Heads configures all supported platforms with this behavior (we want similar configs for all boards upstream)
- Offer a configuration toggle to control it for now - I will want to have better integration with our coreboot utility and other tooling before I enable it in PureBoot (they need to tell the user what to do, can't just fail in confusion). (I'm happy to help with this too.)
- Offer a configuration toggle to control it for now
It's already there as per original PR0 PR: as of now, user can turn it off for one boot under config settings toggle. @JonathonHall-Purism what would you suggest instead?
Per OP workflow screenshots:
@tlaurion @miczyg1 Nice work here, I'm thrilled to see this. I hope we can get it to work 🤞
What SoCs has this been tested on so far? I applied the coreboot patch to Purism's coreboot fork and built this for the Librem 14; the OS is still able to write to the firmware. The coreboot patch is implemented for cannonlake but I don't know if that was tested.
I did see
Finalizing chipset Write Protection through SMI PR0 lockdown call, so the config is right and Heads did appear to try to lock flash. (Full disclosure, I booted manually withbasic-autoboot.shfrom the recovery shell, but this uses the samekexec-bootpath and I did see that trace.)The OS was still able to write to flash (in this case I changed the serial number using our coreboot utility: that reads flash, uses cbfstool to change the serial, then writes flash).
I haven't looked into it any further. What SoCs have you all tested? I'd be happy to help get this working for CNL and other SoCs but I probably will not be able to do it prior to the feature freeze.
Question here is what do we want prior of merging @miczyg1 @JonathonHall-Purism. All Skylake+ platforms implementing PR0?
I'd suggest:
- Heads configures all supported platforms with this behavior (we want similar configs for all boards upstream)
- Offer a configuration toggle to control it for now - I will want to have better integration with our coreboot utility and other tooling before I enable it in PureBoot (they need to tell the user what to do, can't just fail in confusion). (I'm happy to help with this too.)
@JonathonHall-Purism: I checked it on Alder Lake and Comet Lake (so using coreboot's soc/cannonlake) and it worked.
tlaurion Edited: tagged Jonathon
@miczyg1 is https://github.com/Dasharo/coreboot/commit/ff22122c229bbe2109de92ded773493428f7ece9 linked to an upstream patchset I've missed for upstream review?
Maybe this would make this go faster? (issue thought unfixable for ~3y even though under vaultboot for skylake, see https://github.com/linuxboot/heads/pull/326#issuecomment-939176320)
It would also be nice to see traction upstream on this for all platforms, including up to meteor lake. This is really good security feature, and tested working for some, but apparently not all for the same family as reported by @JonathonHall-Purism
@miczyg1 is Dasharo/coreboot@ff22122 linked to an upstream patchset I've missed for upstream review?
Not yet. Sorry about that. It completely slipped my mind. Will push ASAP.
@miczyg1 is https://github.com/Dasharo/coreboot/commit/ff22122c229bbe2109de92ded773493428f7ece9 linked to an upstream patchset I've missed for upstream review?
Maybe this would make this go faster? (issue thought unfixable for ~3y even though under vaultboot for skylake, see https://github.com/linuxboot/heads/pull/326#issuecomment-939176320)
It would also be nice to see traction upstream on this for all platforms, including up to meteor lake. This is really good security feature, and tested working for some, but apparently not all for the same family as reported by @JonathonHall-Purism
@tlaurion thank you for persistently pushing for upstreaming/ contributing back to main coreboot tree. The community thanks you ☺️
bumped to last changes over coreboot pointed patchset, rebased on master under latest commit.
@miczyg1 I replied to @paulmenzel at https://review.coreboot.org/c/coreboot/+/85278/comment/1bc100aa_c7700a9d/
@tlaurion thank you for persistently pushing for upstreaming/ contributing back to main coreboot tree. The community thanks you ☺️
No worries @LeanSheng , it was supposed to go upstream from the beginning. It was just simpler for me to quickly draft a PoC on a fork and test on HW I had with me locally.
@miczyg1 Cannot use patchset against current dasharo fork used by nv41, so will revert last commit until either Dasharo bumps the Pr0 path that works for dasharo fork or bumps dasharo fork so that upsteam pathc applies cleanly here
Logs at https://app.circleci.com/pipelines/github/tlaurion/heads/3009/workflows/70ac4899-04e1-4527-883b-f6eea5070c0d/jobs/58411/parallel-runs/0/steps/0-102
@macpijan cc
@miczyg1 Cannot use patchset against current dasharo fork used by nv41, so will revert last commit until either Dasharo bumps the Pr0 path that works for dasharo fork or bumps dasharo fork so that upsteam pathc applies cleanly here
I would focus on testing the boards using upstream coreboot in heads. When we reach the final shape of the patch on gerrit and get it merged, I can cherry-pick it to Dasharo and test the remaining boards based on Dasharo fork.
nv41 is not part of upstream, if you mean to use upstream for this board
I meant all boards that do not use coreboot forks in heads.
nv41 is not part of upstream, if you mean to use upstream for this board
I meant all boards that do not use coreboot forks in heads.
@miczyg1 : there is no >=skylake boards under Heads not depending on coreboot forks.... @LeanSheng/@pietrushnic/@felixsinger/@JonathonHall-Purism: this is something to meditate on.
Boards depending on coreboot master (<skyake):
user@heads-tests-deb12-nix:~/heads$ grep -Rn "CONFIG_COREBOOT_VERSION=24.02.01" boards/
boards/UNTESTED_t440p-maximized/UNTESTED_t440p-maximized.config:7:export CONFIG_COREBOOT_VERSION=24.02.01
boards/x220-hotp-maximized/x220-hotp-maximized.config:12:export CONFIG_COREBOOT_VERSION=24.02.01
boards/x230-maximized/x230-maximized.config:10:export CONFIG_COREBOOT_VERSION=24.02.01
boards/qemu-coreboot-fbwhiptail-tpm1-prod/qemu-coreboot-fbwhiptail-tpm1-prod.config:6:export CONFIG_COREBOOT_VERSION=24.02.01
boards/qemu-coreboot-whiptail-tpm1-hotp-prod/qemu-coreboot-whiptail-tpm1-hotp-prod.config:8:export CONFIG_COREBOOT_VERSION=24.02.01
boards/qemu-coreboot-whiptail-tpm1-prod/qemu-coreboot-whiptail-tpm1-prod.config:6:export CONFIG_COREBOOT_VERSION=24.02.01
boards/qemu-coreboot-whiptail-tpm2-prod/qemu-coreboot-whiptail-tpm2-prod.config:6:export CONFIG_COREBOOT_VERSION=24.02.01
boards/w530-maximized/w530-maximized.config:11:export CONFIG_COREBOOT_VERSION=24.02.01
boards/optiplex-7010_9010-hotp-maximized/optiplex-7010_9010-hotp-maximized.config:10:export CONFIG_COREBOOT_VERSION=24.02.01
boards/t430-hotp-maximized/t430-hotp-maximized.config:10:export CONFIG_COREBOOT_VERSION=24.02.01
boards/qemu-coreboot-whiptail-tpm2-hotp-prod/qemu-coreboot-whiptail-tpm2-hotp-prod.config:7:export CONFIG_COREBOOT_VERSION=24.02.01
boards/qemu-coreboot-fbwhiptail-tpm2/qemu-coreboot-fbwhiptail-tpm2.config:6:export CONFIG_COREBOOT_VERSION=24.02.01
boards/qemu-coreboot-whiptail-tpm1/qemu-coreboot-whiptail-tpm1.config:6:export CONFIG_COREBOOT_VERSION=24.02.01
boards/qemu-coreboot-fbwhiptail-tpm2-hotp/qemu-coreboot-fbwhiptail-tpm2-hotp.config:7:export CONFIG_COREBOOT_VERSION=24.02.01
boards/x230-hotp-maximized/x230-hotp-maximized.config:10:export CONFIG_COREBOOT_VERSION=24.02.01
boards/optiplex-7010_9010-maximized/optiplex-7010_9010-maximized.config:10:export CONFIG_COREBOOT_VERSION=24.02.01
boards/t420-hotp-maximized/t420-hotp-maximized.config:12:export CONFIG_COREBOOT_VERSION=24.02.01
boards/qemu-coreboot-fbwhiptail-tpm1-hotp/qemu-coreboot-fbwhiptail-tpm1-hotp.config:8:export CONFIG_COREBOOT_VERSION=24.02.01
boards/qemu-coreboot-fbwhiptail-tpm2-prod/qemu-coreboot-fbwhiptail-tpm2-prod.config:6:export CONFIG_COREBOOT_VERSION=24.02.01
boards/qemu-coreboot-fbwhiptail-tpm1/qemu-coreboot-fbwhiptail-tpm1.config:6:export CONFIG_COREBOOT_VERSION=24.02.01
boards/x230-maximized-fhd_edp/x230-maximized-fhd_edp.config:22:export CONFIG_COREBOOT_VERSION=24.02.01
boards/x230-hotp-maximized-fhd_edp/x230-hotp-maximized-fhd_edp.config:22:export CONFIG_COREBOOT_VERSION=24.02.01
boards/x230-hotp-maximized_usb-kb/x230-hotp-maximized_usb-kb.config:12:export CONFIG_COREBOOT_VERSION=24.02.01
boards/UNTESTED_w541-maximized/UNTESTED_w541-maximized.config:7:export CONFIG_COREBOOT_VERSION=24.02.01
boards/x220-maximized/x220-maximized.config:12:export CONFIG_COREBOOT_VERSION=24.02.01
boards/qemu-coreboot-whiptail-tpm2-hotp/qemu-coreboot-whiptail-tpm2-hotp.config:7:export CONFIG_COREBOOT_VERSION=24.02.01
boards/w530-hotp-maximized/w530-hotp-maximized.config:11:export CONFIG_COREBOOT_VERSION=24.02.01
boards/optiplex-7010_9010_TXT-maximized/optiplex-7010_9010_TXT-maximized.config:10:export CONFIG_COREBOOT_VERSION=24.02.01
boards/t430-maximized/t430-maximized.config:10:export CONFIG_COREBOOT_VERSION=24.02.01
boards/qemu-coreboot-whiptail-tpm2/qemu-coreboot-whiptail-tpm2.config:6:export CONFIG_COREBOOT_VERSION=24.02.01
boards/t530-maximized/t530-maximized.config:11:export CONFIG_COREBOOT_VERSION=24.02.01
boards/qemu-coreboot-fbwhiptail-tpm1-hotp-prod/qemu-coreboot-fbwhiptail-tpm1-hotp-prod.config:8:export CONFIG_COREBOOT_VERSION=24.02.01
boards/t530-hotp-maximized/t530-hotp-maximized.config:11:export CONFIG_COREBOOT_VERSION=24.02.01
boards/t420-maximized/t420-maximized.config:11:export CONFIG_COREBOOT_VERSION=24.02.01
boards/z220-cmt-maximized/z220-cmt-maximized.config:28:export CONFIG_COREBOOT_VERSION=24.02.01
boards/qemu-coreboot-fbwhiptail-tpm2-hotp-prod/qemu-coreboot-fbwhiptail-tpm2-hotp-prod.config:7:export CONFIG_COREBOOT_VERSION=24.02.01
boards/optiplex-7010_9010_TXT-hotp-maximized/optiplex-7010_9010_TXT-hotp-maximized.config:10:export CONFIG_COREBOOT_VERSION=24.02.01
boards/qemu-coreboot-whiptail-tpm1-hotp/qemu-coreboot-whiptail-tpm1-hotp.config:8:export CONFIG_COREBOOT_VERSION=24.02.01
Boards >=skylake depending on coreboot forks:
user@heads-tests-deb12-nix:~/heads$ grep -Rn "CONFIG_COREBOOT_VERSION=" boards/ | grep -v 24.02.01 | grep -v 4.11 | grep -v talos
boards/nitropad-ns50/nitropad-ns50.config:5:export CONFIG_COREBOOT_VERSION=dasharo
boards/librem_11/librem_11.config:6:export CONFIG_COREBOOT_VERSION=purism
boards/librem_13v4/librem_13v4.config:6:export CONFIG_COREBOOT_VERSION=purism
boards/librem_15v4/librem_15v4.config:6:export CONFIG_COREBOOT_VERSION=purism
boards/librem_13v2/librem_13v2.config:6:export CONFIG_COREBOOT_VERSION=purism
boards/librem_l1um_v2/librem_l1um_v2.config:6:export CONFIG_COREBOOT_VERSION=purism
boards/librem_15v3/librem_15v3.config:6:export CONFIG_COREBOOT_VERSION=purism
boards/novacustom_nv4x_adl/novacustom_nv4x_adl.config:5:export CONFIG_COREBOOT_VERSION=dasharo
boards/librem_mini_v2/librem_mini_v2.config:6:export CONFIG_COREBOOT_VERSION=purism
boards/librem_mini/librem_mini.config:6:export CONFIG_COREBOOT_VERSION=purism
boards/librem_14/librem_14.config:6:export CONFIG_COREBOOT_VERSION=purism
so @miczyg1 @macpijan @JonathonHall-Purism following this reasoning, Purism and Dasharo forks need to include the patch, adapted to coreboot base they depend on under patches/coreboot-purism and patches/coreboot-dasharo-unreleased to be tested under Heads by willing testers. Of course, Heads will need to bump used coreboot release at some point, but that is irrelevant to #1821 here.
TLDR: @macpijan @miczyg1 @pietrushnic: if you want PR0 to be part of the features of next release, the upstream patch needs to either be adapted to Dasahro fork(s): nv41/ns50/v540TU/v560TU( ideally: one dasharo coreboot port containing what is needed for all dasharo firmware supported boards, not many)
I repeat: Next coreboot version bump for all boards mentioned above bump to something newer than 24.02.01 won't affect Pr0 for skylake+, simply because none of the boards under Heads using coreboot releases are skylake+: all skylake+ boards are supported under forks
I would focus on testing the boards using upstream coreboot in heads.
@miczyg1 (dasharo) @JonathonHall-Purism (purism) forks: TLDR: from above comment this is not possible today because no >=skylake boards are using coreboot master today, therefore patches needs to be in forks ot be tested.
A reminder that Heads is a user of downstream/upstream coreboot forks. Not a coreboot fork maintainer. Nothing much I can do here.
@JonathonHall-Purism @miczyg1 As far as I'm concerned, this PR is ready to merge as it is (patch against nv4x_adl currently used dasharo coreboot fork+unrealeased patch), is in a functional state, until downstream adapts the patch in their downstream fork. This is not my fight. Updated OP.
Waiting for comments otherwise merging once this builds.
- [x] Test on nv41_adl : @tlaurion
- [x] merge
PR0 needs porting of upstream patch to downstream forks
@nestire @daringer @jans23: Removing ns50 lock_chip because I cannot test. Simple to add, just revert next commit in a PR after testing it and I will merge.
@JonathonHall-Purism patch needed on coreboot purism fork
All downstream coreboot forks: Please refer to #1818 for how to enable PR0 from board config and config/coreboot* config files if not using upstream, adapt https://review.coreboot.org/c/coreboot/+/85278 and create PR upstream (under https://github.com/linuxboot/heads) prior of #1821 if you intend to have the feature before freeze for your platforms.
TLDR: from above comment this is not possible today because no >=skylake boards are using coreboot master today, therefore patches needs to be in forks ot be tested.
Ugh... I was not aware of that. Allright. Then I will focus on getting the patch merged upstream, then cherry-pick to dasharo.
TLDR: from above comment this is not possible today because no >=skylake boards are using coreboot master today, therefore patches needs to be in forks ot be tested.
Ugh... I was not aware of that. Allright. Then I will focus on getting the patch merged upstream, then cherry-pick to dasharo.
@miczyg1 it has been awhile, any update on coreboot patch?
TLDR: from above comment this is not possible today because no >=skylake boards are using coreboot master today, therefore patches needs to be in forks ot be tested.
Ugh... I was not aware of that. Allright. Then I will focus on getting the patch merged upstream, then cherry-pick to dasharo.
@miczyg1 it has been awhile, any update on coreboot patch?
@miczyg1 there is merge conflict https://review.coreboot.org/c/coreboot/+/85278
Ideally, links to downstream commits and versions of coreboot should be clarified under that upstream merge request I guess for repro?
Not sure how to make this merged upstream here. The number of forks of coreboot is problematic to say the least @LeanSheng
Under heads and as merged for this PR:
- This patch https://github.com/linuxboot/heads/pull/1818/files#diff-7021ca62952438efd0f02f544f4566f098e2c7852164a82aeeca0efdb80cf2aa
- merged under master https://github.com/linuxboot/heads/blob/fa0f90cbecfd4b8e545c2a77bc5338e033652f2c/patches/coreboot-dasharo-unreleased/0002-pr0_chipset_locking-post_skylake.patch#L1-L14
- Applied atop of this dasharo fork commit : https://github.com/linuxboot/heads/blob/fa0f90cbecfd4b8e545c2a77bc5338e033652f2c/modules/coreboot#L94-L98
@tlaurion yes you are right. thats y we always ask people to upstream to coreboot directly and not to create another fork and keep things there. coreboot is not the same as linux kernel which has big ecosystem; but rather niche and need more support for growth, especially for companies that using coreboot for business should be uphold bigger responsibilities for upstreaming and maintaining. for many years we have been working hard to promote coreboot, have to work together.
@miczyg1 it has been awhile, any update on coreboot patch?
I will try to find some time to push new patchset.
@miczyg1 Merry Christmas and Happy New Year! Any update? What is the plan to update it?