-
-
Notifications
You must be signed in to change notification settings - Fork 187
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
add google parrot config #966
base: master
Are you sure you want to change the base?
Conversation
@gompa There was a long debate in the past under Heads, where it was decided that ifd could be distributed, where ME couldn't if not already part of the coreboot's blobs and or if the binary redistribution clauses were not clear on that effect. The idea is that Heads git repository doesn't want to be the redistributor of such blobs. |
but the ME and ifd are in the normale coreboot repo : https://github.com/coreboot/blobs/tree/master/mainboard/google/parrot, this is just a nuterd version for more space, but i can remove them if needed |
Seems like those are redistributable blobs! |
@gompa Please run the following in head's recovery shell and post the output to check everything is being measured okay.
|
second command doesn't return anything so i guess not ? PCR-00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |
@gompa I would suggest, sincerely and for simplicity, to simply copy paste the code of other blob extraction scripts and implement download and verification logic on hashes obtained from a downloaded blob in repo for a specific commit, and reapply the logic of me_cleaner and verify the hashes of produced blobs dumped inside of your blobs dir, where coreboot config will look for the blobs at each build. Documenting those blobs inside of the board config would also be appreciated. And the build failing because of blobs not existing in tree would both respect the blobs non-redistribution clause, dodging legaility problems, while permitting other users to build easily. The idea here is that if you implement this in this PR and the extraction tool can be called from command line, your board could then be added into CircleCI's builds, calling your script to download the blobs, neuter them and produce modified IFD matching the verified measurements you have taken to reproduce the same ROM. So each time a new commit lands into master, your board, as all other in CircleCI, will be rebuilt and ROMs produced. Endgoal of this is to have fwupd upgrades in the longer run, when CircleCI builds will have some logic added to be able to validate other builds from other instances and the hashes validated, prior of being uploaded into github releases, and github releases, ingested by LVFS so users can have upgrades of Heads upon new releases. |
This will require further investigation. PCR2 should have measured Boot block, ROM stage, RAM stage, Heads linux kernel and initrd, so the fact its completely empty is quite concerning. |
after adding : CONFIG_TPM_MEASURED_BOOT=y
|
@gompa sorry can you just confirm that |
yes ! :
|
this should be done now, please let me know if anything else needs changing |
@@ -0,0 +1,53 @@ | |||
# Configuration for a Parrot running Qubes and other OSes | |||
# |
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.
A note here on how you obtained and reproduced the shrinked ifd would be helpful for people wanting to reproduce.
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.
@gompa would still be required!
#SSH client/server | ||
CONFIG_DROPBEAR=n | ||
#Ethernet driver (Heads only) | ||
CONFIG_LINUX_E1000E=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.
Havent checked the specs of platform but no GBE in rom means that creating a full rom like we do here with ME and ifd results in the equivalent of a maximized board.
Putting ME and creating an expended ifd taking advantage of the freed space means that the IFD put into repo is unlocked with ifdtool. is that the case? If so it should be documented properly, stating how the result was obtained.
If such, the flashrom upgrade line into board config should remove --ifd --image BIOS
as in maximized boards configs
Question is: you were successful flashing it externally with ifd and ME included in coreboot image? If there is no Ethernet port (I read eth0 somethwere... No need for a gbe blob?), and then no GBE region needed here, then it is candidate for maximized board and the flash options below should be changed accordingly. If there is eth0, its not e1000e based?
Otherwise, if IFD was not unlocked (for some reason) and we do not want to be able to upgrade ME later on or reflash whole SPI region internally, leave it be with specifying --ifd --image BIOS below. This means that on firmware future internal firmware upgrades, only the BIOS region (fixed in both ifd provided matching CBFS_REGION in coreboot config) will be modifiable. If ME is upgraded in repo later on, and IFD was not unlocked, it means that external reflashing would be required to write ifd, ME changing size.
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.
Question is: you were successful flashing it externally with ifd and ME included in coreboot image? If there is no Ethernet port (I read eth0 somethwere... No need for a gbe blob?), and then no GBE region needed here, then it is candidate for maximized board and the flash options below should be changed accordingly. If there is eth0, its not e1000e based?
yeah i was succesfull flashing it externaly, first time i flashed it internaly i "bricked" it. (probably locked ifd)
the nic is a tg3 broadcom card and i dont think it needs a gbe blob
Otherwise, if IFD was not unlocked (for some reason) and we do not want to be able to upgrade ME later on or reflash whole SPI region internally, leave it be with specifying --ifd --image BIOS below. This means that on firmware future internal firmware upgrades, only the BIOS region (fixed in both ifd provided matching CBFS_REGION in coreboot config) will be modifiable. If ME is upgraded in repo later on, and IFD was not unlocked, it means that external reflashing would be required to write ifd, ME changing size.
the ifd is not unlocked mostly because i forgot to unlock it and flashed it externaly , i could unlock it ? i dont know what's prefered
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.
@gompa: Other boards (non maximized) take into consideration that blobs are extracted from backups (could be another script called extract, as in x230 maximized blobs dir), where the script calls ifdtool -u by default, where the user can decide for himself to not unlock it.
If the goal is to have maintained fwupd board, where a new commit of Heads could lead to new ME bin in coreboot in newer coreboot version, then only unlocked ifd with corrected new maximized free space + updated ME (from unlocked region) would permit such upgrade. (Code is pending review under QubesOS and FWUPD as of right now, where helper script will need to be developped under Heads to verify and ask user to flash on later version.)
Not unlocking ifd means that the board config would need to stay as is only specifying BIOS to be flashable, where future ME upgrade (if that is even a thing, I have not researched this board at all) would require the board to be externally reflashed. Is there a screw that needs to be unscrewed to flash SPI? I have no idea here, so I leave you the choice of implementation.
In all case, if we want a fully upgradeable ROM from fwupd, it would need ifd to be unlocked.
There is no consensus on this as of now and there was debates on the subject before around, if you look. My opinion is that IFD should be unlocked so whole SPI can be inspected/upgraded.
export CONFIG_BOOT_KERNEL_REMOVE="quiet" | ||
export CONFIG_BOOT_DEV="/dev/sda2" | ||
export CONFIG_BOARD_NAME="Parrot" | ||
export CONFIG_FLASHROM_OPTIONS="--force --noverify-all -p internal --ifd --image bios" |
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.
Change to "--force --noverify-all -p internal" if BIOS image is complete, and no GBE is needed (so that we can internally reflash everything from within Heads, the whole SPI flash; not just the BIOS region).
If GBE image needed, another extraction script needs to be put in blobs directory, so that the user has to run it to extract his GBE from original rom backup, and coreboot config needs to depend on it.
Otherwise, we have another maximized board, where CBFS region should be reviewed in coreboot config, and ifd reviewed one last time to make sure that BIOS region is taking all the space left from ME, considering your ifd is unlocked with ifdtool and the documentation is added into board config to replicate what you have done to be able to provide such ifd. (See x230-maximized blobs dir documentation for example)
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.
On X230, the absence of GBE makes the initialization of the ethernet card impossible, since the MAC address is defined under GBE blob. That is why I ask so many questions about it... And to document this board's PR so others can review steps needed to produce a functional port inside of Heads. #700
Thanks for your contribution!
@gompa another review round in comments! |
@gompa One nice to have addition for added board not needing external blobs or having redistribution of blobs issue (like here) is to have a CircleCI entree added into .circleci/config.yml:
Then if you are following your github project from CircleCI, the build will happen on next commit, so you can test built ROM from there. If its functional, we will be able to merge and other Parrot users will be happy. Are you ok to be added into #692 ? |
@tlaurion ah cool wil have to check that out ! sure you can add me to #692. |
@gompa running qubes-hcl-report would resolve the doubts with clear viww out of dom0 console over qubesos. Otherwise, its just speculations. |
when booting the qubes installer from usb it gives an error about unsupported hardware, missing features IOMMU/VT-d/AMD-Vi,Interupt Remapping |
boards/Parrot/Parrot.config
Outdated
@@ -0,0 +1,53 @@ | |||
# Configuration for a Parrot running Other OSes, No QubesOS support since no vt-d support! | |||
# | |||
# Deactivated to fit in coreboot's CONFIG_CBFS_SIZE=0x700000 : |
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.
doestn match coreboot config line
@tlaurion i cleaned up the board config, could you please check if the ci config is correct like this, its a direct copy of the snippet above but i cant test |
@gompa Sorry. Got lost in time. Created branch https://github.com/tlaurion/heads/tree/gompa_parrot which should trigger the CircleCI build https://app.circleci.com/pipelines/github/tlaurion/heads/717/workflows/d2423e2b-21a3-4a7f-b86c-fb0a55981d85/jobs/864 But CircleCI builds are foo bar for the moment. Too many boards and parallelization is needed at this point. |
@gompa meanwhile, Heads community in not really loving PRs with many commits (I do. I think this board is the exact example of how to add a new boards into Heads). @MrChromebox : Would you be ok with merging so many commits directly? |
@tlaurion absolutely not, this definitely needs cleanup. Additionally, I'm not certain this will work for all Parrot boards given there are both SNB/IVB versions, which I believe use different ME firmware. I can get away with it for my MrChromebox UEFI firmware since I don't reflash the ME |
CONFIG_TPM_RDRESP_NEED_DELAY=y | ||
CONFIG_UART_PCI_ADDR=0 | ||
CONFIG_NO_GFX_INIT=y | ||
CONFIG_CONSOLE_CBMEM_BUFFER_SIZE=0x80000 |
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.
CONFIG_UART_PCI_ADDR=0
CONFIG_CONSOLE_CBMEM_BUFFER_SIZE=0x80000
CONFIG_DEFAULT_CONSOLE_LOGLEVEL_5=y
these aren't needed
CONFIG_BOARD_GOOGLE_PARROT=y
is duplicated
# CONFIG_DRIVERS_UART_8250IO is not set
should be added
@@ -0,0 +1,302 @@ | |||
CONFIG_LOCALVERSION="-heads" |
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.
why parrot-common
? I'd drop -common
device pnp ff.1 on # dummy address | ||
end | ||
end | ||
+ chip drivers/pc80/tpm |
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.
this should be upstreamed to coreboot
@gompa ping! |
@gompa Current state of this PR prior of merge:
|
enable google parrot in heads
add coreboot and linux config and patch to enable tpm0