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

xx20 blobs fix #912

Merged
merged 4 commits into from
Dec 3, 2020
Merged

Conversation

Thrilleratplay
Copy link
Contributor

Replacement for PR #877

  • Added GBE blob
  • Added IFD blob already modified for minified ME
  • added me7_update_parser.py (more information on what it does) script which creates a minified cleaned version of ME7 from the xx02 ME update file.
  • updates download script to automate download of ME update, extraction and processing.
  • replaced consolidated t420 and x220 blob directories under blobs\xx20
  • Added whiptail gui and hotp from x230 board config.
  • Updated circleci build

I have externally flashed the artifact from my cicleci build 18 on a x220 and it boots to the Heads gui.

Copy link
Collaborator

@tlaurion tlaurion left a comment

Choose a reason for hiding this comment

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

@Thrilleratplay small required changes

boards/t420/t420.config Outdated Show resolved Hide resolved
blobs/xx20/readme.md Show resolved Hide resolved
Copy link
Collaborator

@tlaurion tlaurion left a comment

Choose a reason for hiding this comment

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

@Thrilleratplay : no more --ifd specified regions here. We want whole flash, both for external flash and internal flash.

This is kind of the goal here :)

We also have to remind new comers that flashing internally this rom IS NOT compatible with prior versions board roms previously compiled.

The technical logic behind this is that we are tempting Murphy otherwise:
A user flashing internally only the BIOS region SHOULD not be possible since the BIOS region is bigger then the maximum IFD region. You can test that here from past xx20 board, and attempt an internal flash. Flashrom might permit (untested) to flash a bigger BIOS region inside of a too small BIOS region, where IFD was not resized properly on initial external flash to have proper ME and BIOS regions setuped correctly.

So we have a choice here:

  • Document, where users will obviously attempt to upgrade internally since the board config is the same and brick their device if flashrom doesnt validate properly. It could permit the flash. From my early experiements with x230-external-flash, flashrom was permitting this kind of mess, while flashrom was upgraded since. Might be a flase problem but we will surprise some users. This is unclear upgrade path.
  • Rename the board with -external-flash (note that coreboot config needs to point to BOARD related build dir, linux config is fine, while all should match for consistency, unless really sharing the same config (where here coreboot CBFS would be different. Unless really common for all boards where a xx20 variant could be introduced, or here again, a xx20-externak-flash.config would be self explainable).

boards/t420/t420.config Outdated Show resolved Hide resolved
boards/x220/x220.config Outdated Show resolved Hide resolved
@tlaurion
Copy link
Collaborator

tlaurion commented Nov 27, 2020

@Thrilleratplay : Not sure about reusing xx20 previous boards definition. They should be phased out and replaced, so past builders will notice directly that they cannot make BOARD=x220 anymore, and realize that a x220-external-flash exists, requiring him to flash externally (since we do not know their IFD state regions and should not imply that they are good).

This is why I named x230-external-flash board, expecting to remove x230 board altogether when the PR is approved. Not sure about this migration path, but I think we have to limit bricks of users who were using xx20 boards before. My thought process here was that if x230 disappears in board dir, that would be a clear, non-documentation needed path for builders to be forced to realize that board changed, where past board phased out. Thoughts welcome.

So what I would recommend is:

  • to have boards renamed x220 -> x220-external-flash and t420 -> t420-external-flash
  • have coreboot config files renamed accordingly, with paths modified accordingly matching BOARD names for payload and initrd paths

@Thrilleratplay
Copy link
Contributor Author

Thrilleratplay commented Nov 27, 2020

@tlaurion While I agree with most of the internal/external flashing comments, there seems to two different points being argued. One where the current user's layout is preserved by creating boards x220-external-flash/t420-external-flash and one where it doesn't matter as the internal flash is updating all regions of the rom.

There does not seem to be a good answer. Currently, the internal flash scripts update only the BIOS region. By using the internal flash scripts with this rom as is, likely will result in an error. Flashing externally, means they will lose their configuration and need to set it up again.

I am thinking a modified version of flash.sh that can be made that can be included on the same USB as this new version of Heads rom, that copying the PGP key into the new CBFS and flashes the entire rom.

@Thrilleratplay
Copy link
Contributor Author

@tlaurion Generally speaking for this PR and #703, I think it is best to consolidate the board variations into single models (if possible). Instead of x230-nkstorecli, x230-hotp-verification, x230-flash, there is only x230 which includes the nkstorecli and hotp options with the IFD, ME and GBE in one 12mb file. An extra task to split it into 8mb and 4mb chunks, complete with sha256, will allow users to either build from scratch or download the artifacts then flash externally on each chip or internally with the full 12mb rom, all with one build. For the x220/t420, it would be the same but there is no need split the file, there is only one 8mb chip.

Initially, this will be a headache as users will need to take special steps to update the entire rom, as the current flash.sh only updates the bios but can be mitigated with a update script (as noted in my previous comment).

@tlaurion
Copy link
Collaborator

tlaurion commented Nov 27, 2020

@Thrilleratplay
I see that you updated the flashrom commands in boards configs. That will be insufficient.

My comments were targeting the following.
1- User is having x220 board compiled from master (or older versions... with older flashrom versions, included.)
2- User builds x220 board/downloads CI image from the result of this merge (which requires external initial flashing)
3- User attempts to flash internally the new ROM. This means he attempts to only flash BIOS region (he is using past build's specified FLASHROM command, or older flash script from past revision, we cannot know), which is supposed to match CBFS coreboot defined space. Which is supposed to match IFD expended BIOS region, resulting of reduced ME region. This is not the case here. Behavior?
4- The user will be able to flash internally, the whole flash, only after having already flashed externally. Brick guaranteed otherwise. Where if x220 and x220-external-flash were separated, and where x220 would be deprecated, and deleted, and replaced with x220-external-flash and x220-hotp-external-flash we would do proper prevention.

I agree all those x230 boards should be merged inside of x230-external-flash. Where the scenario is more uncertain for the xx20 boards. This is where specialization of boards started for the different x230 boards for space constraints and use cases differences

Once again:

  • x230 actual board does not include HOTP support. Adding HOTP support requires, in code paths, to have a USB Security dongle supporting HOTP. This is not common to all use cases. x230-external-flash currently contains HOTP (USB Security dongle remote administration), NKSTORECLI (For new reownership mode i'm working on but not fast enough in my taste), dropbear, ethernet drivers etc since there is no space constraints for the moment. Further versions may differenciate the builds, while this makes me realize I should differenciate htop and non-hotp boards myself since htop requires a HOTP enforcing USB Security dongle, where x230 requires a USB Security dongle (a GPG dongle) without HOTP.

I think the paths should be different and should differentiate those use cases too:

  • We should not force HOTP based remote attestation based on USB Security dongle upon users. TPM based TPMTOTP is perfectly fine for most tech people not desiring to buy a USB Security dongle.

I just know that replacing the x220 doesn't seem the good path.
For your script, its a chicken/egg problem. Since we do not know if user really unlocked ifd with ifdtool, nor neutered ME correctly (might have just deactivated it without neutering it) we cannot imply that IFD matches reduced ME region and expended BIOS region, nor that we can overwrite IFD internally, unless the first flash was external with such board produced ROM. Even the idea of adding a new script to force full internal reprogramming would not guarantee that the user followed proper instructions (seen many times in different tickets for x230 blank screen, t40 shutting down, where were tickets linked to inconsistencies between IFD, ME size and BIOS size regions.)

This is why I would phased out xx20 board for their xx20-external-flash counterparts, and would continue to have -htop counterparts, since they imply different use cases and code paths, not just size related issues nor prior external flash.

Let's chat live?

@Thrilleratplay
Copy link
Contributor Author

@tlaurion Ok, I think I understand what you are try to accomplish. xx20-external-flash boards are intended to be used as externally flashed roms in addition to x220, the same way x230 and x230-flash are built together (I just noticed that in #703 that those are x230-external-flash and t430-external-flash). It is a new board to distinguish it from the previous layout. That makes sense if the other variants are being removed. My only complaint is the names x230-external-flash and x230-flash are too similar, what about xx30-external-flash was something along the lines of xx30-complete-rom? The xx20 boards can follow the same nomenclature.

As for HOTP, if it needs to be a separate board build, that is fine. I assumed it wasn't included due to space restrictions and was being readded based on CONFIG_HOTPKEY=y being added toto the xx30-external-flashboards.

@tlaurion
Copy link
Collaborator

tlaurion commented Nov 27, 2020

@Thrilleratplay

If we move away of x230-flash and x230 bundle, their counterparts could continue to be named x230-external-flash since there wouldn't be any nomenclature imprecision, since both boards (x230-flash and x230-external-flash) would be short lived and hopefully tested as a good replacement for x230 owners (where official documentation instructed to unlock ifd, so we could safely imply that the user can actually follow an upgrade path from x230 to x230-external-flash by flashing internally with the proper flash command not specifying --ifd BIOS region.

The plan is to phase out two parts boards altogether, while enforcing clear distinction that one are not compatible with the other.

As for -hotp boards, they implement and bind the user to have bought another piece of hardware, which we do not want to render a prerequisite for Heads. It actually and simply replace TPMTOTP remote attestation on smarphone dependency into looking that your USB Security dongle is flashing green for the same result.

As for implementing a generic xx30 board, there are differences on the coreboot config level for the t430 and x230 since the t430 has integrated graphics + external GPU, and there was requests to enforce those differences with OPTIONAL_ROM support if I recall well. Once again, since I do not own those boards, its hard for me to tell if they should be joined or not. I just recall that I was against merging those changes inside of the x230 since they were not relevant.

So my recommendation stays to:

  • rename (at least, not duplicate. Not necessary) x220 -> x220-external-flash
    The goal being that once your PR is merged, x220 board would die (if we want to enforce it).
    Otherwise:
  • Duplicate board and make x220 die soon, since lack of BIOS region space already faced. (note that Upgrade to cryptsetup 2.3 and make cryptsetup1/cryptsetup2 optionals #876, encompassed in TPM2 support #893, required to boot installed QubesOS 4.1 (LUKS2) will require more space sooner then later... Which was also not fitting in x230 board without ME neutered+deactivated.)

For the rest, I envision interest into having x220-hotp-external-flash board series. Where we could have all modules dependencies included, until space problems arises again and we will need, then, to specialize boards accordingly. Users of the same "labeled board" will be able to update external-flash boards internally. Where switching in between boards will require external flashing.

@tlaurion
Copy link
Collaborator

tlaurion commented Nov 28, 2020

( When we agree on nomenclature, I will modify xx30 PR accordingly. Same logic applying to all xx20 and xx30, and will even try to coordinate merging of those two pull requests.)

Considering this, I would not mv board files but would create new ones (x220-external-flash, t420-external-flash, x230-external-flash, t430-external-flash, x220-hotp-external-flash, t420-hotp-external-flash, x230-hotp-external-flash, t430-hotp-external-flash).
I would THEN create another PR to deprecate them (x220, t420, x230-hotp, t430-hotp, others) soon after.

This would be first step before integrating #876 or attempting #562 inclusion and debugging, which would better work on top of this anyway.

@tlaurion
Copy link
Collaborator

@merge Lazy here sorry to poke you. But in Skulls instructions, do you FORCE users to unlock IFD? Could a simple instruction be pushed to current users that if they intend to flash x230-external-flash, t430-external-flash, x220-external-flash t420-external-flash and all other future coming sandy / ivy bridge based upraders have to flash with a manual command from command line not only flashing BIOS region but whole flash?

@tlaurion
Copy link
Collaborator

tlaurion commented Nov 28, 2020

Note to myself, before merging both xx20 and xx30 branches, ethernet bringup script should randomize MAC address from Intel OUI range prior of trying to enable link.

@tlaurion
Copy link
Collaborator

@Thrilleratplay hmmm your PR gots corrupted. bad rebase upon master?

@tlaurion
Copy link
Collaborator

Also

#!/bin/bash -eo pipefail
rm -rf build/x220-external-flash/* build/log/* && make CPUS=4 V=1 BOARD=x220-external-flash || touch /tmp/failed_build

2020-11-28 04:12:45+00:00 Wrong gawk detected: 
Makefile:109: *** /root/project/boards/x220-external-flash/x220-external-flash.config: board configuration does not exist.  Stop.

CircleCI received exit code 0

@Thrilleratplay
Copy link
Contributor Author

@tlaurion Huh. Not sure how that happened but rebased against heads/master

@tlaurion
Copy link
Collaborator

tlaurion commented Nov 28, 2020

@merge Lazy here sorry to poke you. But in Skulls instructions, do you FORCE users to unlock IFD? Could a simple instruction be pushed to current users that if they intend to flash x230-external-flash, t430-external-flash, x220-external-flash t420-external-flash and all other future coming sandy / ivy bridge based upgraders have to flash with a manual command from command line not only flashing BIOS region but whole flash?

@n4ru Same question would apply to users coming from 1vyrain path. The goal here is to deprecate x230 and x220 (all xx30 and xx20 boards) which relies on unmodified IFD which specifies BIOS region that is too minimalist. As we discussed precedently in other 1vyrain tickets, there is no possibility to trick environment to flash modified IFD and ME regions with neutered+deactivated+reduced ME space, have ifd modified accordinly so that BIOS region is maximized. (This PR and #703 produces roms containing such ifd, downloaded+neutered+minimized ME with BIOS region expended to 11.5mb for the xx30 and 7.5Mb for the xx20).

What would be your thoughts and migration paths? Consequential xx20-maximized and xx30-maximized boards on Heads will arrive soon, requiring externally flashed unlocked IFD to be able to wholly internally flash full featured rom upgrades, also from fwupd (LVFS), where current xx30 and xx20 board configs instructs flashrom to only flash --ifd bios regions ( only --ifd bios region as opposed to current boards, having ifd unlocked, me neutered+reduced region and expended BIOS region and the ROM being wholly internally flashable ). As you know, freeing ME liberates an additional ~5mb (passing from 7mb to ~11.5 of available space on xx30 and passing to 7.5mb available BIOS region space on xx20).

@merge @n4ru: Consequently. Will you issue a big fat warning to users for which external reprogramming would be necessary for users to migrate to the xx20-maximized and xx30-maximized boards produced roms without bricking their machine?

@merge
Copy link
Contributor

merge commented Nov 29, 2020

@merge Lazy here sorry to poke you. But in Skulls instructions, do you FORCE users to unlock IFD? Could a simple instruction be pushed to current users that if they intend to flash x230-external-flash, t430-external-flash, x220-external-flash t420-external-flash and all other future coming sandy / ivy bridge based upraders have to flash with a manual command from command line not only flashing BIOS region but whole flash?

no the skulls "external" install scripts don't force IFD unlocking. They have the -u switch for that.

@tlaurion
Copy link
Collaborator

tlaurion commented Dec 2, 2020

@Thrilleratplay last rebase requested, #703 was merged!

@Thrilleratplay
Copy link
Contributor Author

@tlaurion Rebased.

@Thrilleratplay Thrilleratplay force-pushed the xx20_blobs_extraction_fix branch 2 times, most recently from 7e3a33e to 5c0ee32 Compare December 2, 2020 23:27
@Thrilleratplay Thrilleratplay force-pushed the xx20_blobs_extraction_fix branch 2 times, most recently from b104bdd to f188ab0 Compare December 3, 2020 04:14
boards/t420-maximized/t420-maximized.config Outdated Show resolved Hide resolved
boards/x220-maximized/x220-maximized.config Outdated Show resolved Hide resolved
@tlaurion
Copy link
Collaborator

tlaurion commented Dec 3, 2020

@Thrilleratplay looks good to me while commits could be squashed a bit more (one commit per board addition, one commit for the scripts, and a commit for circleci, where xx20 boards should all be all in the same area for readability and to ease understanding of what the CI does.)

The smaller SPI chips boards should be first in CI so that those builds fail first when someone attempts to add something that breaks those first for quick awareness.

We would be ready to merge :)

@Thrilleratplay
Copy link
Contributor Author

@tlaurion Squashed.

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.

3 participants