-
-
Notifications
You must be signed in to change notification settings - Fork 185
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 Talos 2 board (OpenPower) #1002
Conversation
@SergiiDmytruk did a regression build against 7efe7ce and it was successful. Everything x86 builds as expected from last commit. I would recommend following your branch from CircleCI so that each commit pushed triggers a CircleCI build for that commit. |
You could change this for something similar to the following and tune to have your boards built first with save_cache. (build_and_persist is costy since saving workstpace and restoring workspaces). You might want to only build talos_2 and talos_2_lite and wipe the rest to speed up testing also if regression testing is not needed in all builds (I think changes that are coming next should probably only happen in linux and coreboot configs and trees, so touching other x86 and testing for regression might not be needed.
|
Or without x86 regression testing this simpler version:
|
@tlaurion, thanks. I left one x86 target to see if CI works in this case. I'm not sure because it's not enough to just replace |
I see.... x86 centric and completely understandable until now :/ Probably better to fix INSTALL by appending TARGET/arch suffix to that directory (while musl-cross path should deal with x86 and power from inclusion of the module def) from now on. Fixating caches to include those new directories into them would also be required to be reused:
|
@SergiiDmytruk your board dir and board names are matching to "talos-2" and "talos-2-le" not "talos_2" and "talos_2_le" which you fixed under https://github.com/osresearch/heads/blob/61002eb088c5819867e12dcdb0d5591bc33a9aca/.circleci/config.yml The last build was manually cancelled, but should have continued the build! |
@tlaurion It wasn't me:
|
@SergiiDmytruk weird. This is an error. Let's see if that build is also suspended. |
@SergiiDmytruk note that old cache is being reused for last successful build with same modules and patches created digests as explained in previous post. Those are saved into workspace now and will be passed to board builds to be reused, so it will probably fail since ./install and ./crossgcc (and build/musl-cross) will be shared between x86 and power. Builds taking more then 3 hours per task are also cancelled per terms and services, which is why we struggle using CIs for Heads. |
@tlaurion CI works again for me after contacting support. Haven't updated how caching is done yet. |
@tlaurion Currently Heads produces kernel and initrd separately. Was embedding of initrd into the kernel ever considered? We probably won't need this for coreboot (skiroot needs it), but I thought I'll bring it up just in case there were attempts and known issues. At the moment I only see this way:
|
initrd is actually what Heads is (outside of the patches applied to linux kernel and patches applied to coreboot (less and less, but still.) Just to make sure, you can see those compressed artifacts under /build/$BOARD dir and under CI. Example artifacts for a built board
Some reverse understanding is currently possible since initrd is built separately:
Let me know if this idea is still applicable following details provided above. |
@tlaurion, thanks for the detailed explanation.
I think everything should be OK as is once coreboot will be able to use Heads as payload. |
7dc12cf
to
c90218d
Compare
I do not think it will be in the near future. The goal for now is to replace
|
eb033f5
to
9008891
Compare
@tlaurion I've re-committed changes and marking this PR for review (but will see if CI passes with all the boards). Below is a summary of the changes since the previous comment. The build eventually started to fail due to mixing of files of different architectures under It's just one board again with BE coreboot and LE everywhere else, which seems to be typical for PowerPC. Because we need skiboot, Heads can now optionally build bundled Linux kernel and the new board asks for it. Patches for coreboot can be dropped after 3mdeb/coreboot#79 is merged. There is now |
fe1d9e3
to
586ddd0
Compare
Finally, CI is green. |
@SergiiDmytruk Not sure about the next steps here. Do we wait to merge once Heads can be used as a payload? I see from here:
Where coreboot image doesn't seem to bundle much. Can you detail the current state of the build? |
It's not clear when that will be the case (i.e., when coreboot will support OPAL). A much closer target is the ability to start
It's small because its payload is
My concern about this PR is mostly about changes to the build system, which is the bulk of it. Do they look OK or something needs to be done differently? |
As a basic regression test, I flashed https://113-380596387-gh.circle-artifacts.com/0/build/x86/x230-hotp-maximized/heads-x230-hotp-maximized-v0.2.0-1047-g586ddd0.rom internally and booted Tails with detached signature file verified successfully.
@SergiiDmytruk: I will review the changes done on the build system, but I do not see anything problematic and it opens the door for integration of other architectures then x86. One consideration for Heads (usability) is to use gui-init instead of generic-init, which dependencies are outlined when comparing qemu-coreboot and qemu-coreboot-fbwhiptail (which requires functional graphic framebuffer under linux config). That means that CAIRO and FBWHIPTAIL modules and dependencies might cause you some problems along the way for a workstation aimed build, but I think this would be the desired outcome of the board to be functionally the same as other boards also testing gui-init implemented policy vs the old generic-init which is less used (and mostly deprecated). Note that for the qemu-coreboot board config here, one could simply replace generic-init script with gui-init and have whiptail do the prompts on the console without fb (uglier) but would also work, in case no graphical framebuffer is chosen/possible (that would also work remotely over console, just like for the kgpe-d16 boards configs what were made: server (whiptail) vs workstation (fbwhiptail), where server is aimed to be accessed over the BMC console for remote attestation). @MrChromebox: See any change you do not like in the build system? |
@tlaurion Switched the board to use Also flashed coreboot and kernel built by Heads, but |
@SergiiDmytruk some notes on your last build https://app.circleci.com/pipelines/github/SergiiDmytruk/heads/71/workflows/9b7ca741-8921-4c71-8200-525d6a151448 for SergiiDmytruk@087b754 which included two patches as opposed to last full build that successfully created a cache.
This is why builds took a little longer, they were only taking coreboot cache from built modules. So a rebuild should take less time then what we see here, which is good news. (was not understanding why it was longer build times, this explains that!) |
As noted under SergiiDmytruk@9879e58 I doubt save_cache https://app.circleci.com/pipelines/github/SergiiDmytruk/heads/72/workflows/a1515fdc-e92b-4276-b35b-ee114ba66de0 will work, since packages dir will contain conflicting files :/ |
Hmmm. We would need a way to skip already applied patches if packages cannot be cached, otherwise patches creating files will fail, just like it is happening under https://app.circleci.com/pipelines/github/tlaurion/heads/1158/workflows/ee43e8e4-d487-43a9-b705-2caa833c7948/jobs/9467?invite=true#step-103-3342 for the already created The actual logic is that decompressed archives have ./packages/package-version_verify file created after file integrity validation, which tells archives to not be decompressed again nor patched on top of it if it was already downloaded, verified and extracted. |
Not sure if the following would resolve the issue, but we could:
@SergiiDmytruk : It is to note that the builds succeeded because ./packages was part of the cache here too. |
#1188 was merged, so two additional boards+coreboot config files need #1009 relative fixes |
As can be seen here, the old cache for |
acb6e78
to
4de2de4
Compare
@SergiiDmytruk please rebase on master where #1009 has been merged. Wondering if I should merge #1201 first prior of your master rebasing? |
4de2de4
to
2dbef7c
Compare
Rebased. Order shouldn't matter. I don't think the two PRs conflict. This one doesn't add any new |
`git reset --hard <commit_hash>` is a no-op when commit_hash is unset
Signed-off-by: Sergii Dmytruk <sergii.dmytruk@3mdeb.com>
This also involves splitting workspaces based on target architecture to avoid severely degrading performance of CI. Signed-off-by: Sergii Dmytruk <sergii.dmytruk@3mdeb.com>
2dbef7c
to
b6b9cb0
Compare
This comment was marked as off-topic.
This comment was marked as off-topic.
@SergiiDmytruk please take a look at https://github.com/tlaurion/heads/tree/replace_util-linux_agetty_busybox_getty which is built on top of this branch. Basically, those two changes:
As a result, we would be not asking all board roms to have a 70kb agetty binary that most do not need to use, where adding getty from busybox costs an additional 1k.
Question of course is to know if it works, build is happening here: https://app.circleci.com/pipelines/github/tlaurion/heads/1181/workflows/500aea6c-a6a9-4009-9251-0e09e24b26b3 |
@SergiiDmytruk feel free to borrow code from tlaurion@143adfd if it is doing its job. I was just a bit against adding util-linux's agetty when busybox could provide the same functionality with near to none footprint. Just include what is needed and force push. |
How would you include
|
@SergiiDmytruk yes, exactly.
|
A more complicated example was for ifdtool under coreboot module, needed to be crosscompiled where ifdtool also needed to be natively compiled for coreboot to continue to be able to pack firmware. Giving example because this one was relative to module binary, not kernel module as under linux. a4b5381#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52R464-R479 |
Signed-off-by: Sergii Dmytruk <sergii.dmytruk@3mdeb.com>
It specifies whitespace-separated list of console devices to run Heads on in addition to the default one. Example for board config: export CONFIG_BOOT_EXTRA_TTYS="tty0 tty1" Signed-off-by: Sergii Dmytruk <sergii.dmytruk@3mdeb.com>
Signed-off-by: Sergii Dmytruk <sergii.dmytruk@3mdeb.com>
Signed-off-by: Sergii Dmytruk <sergii.dmytruk@3mdeb.com>
b6b9cb0
to
d0ef7e8
Compare
This provides a configuration for OpenPower that builds (wasn't fully tested yet). It seems to be the first non-x86 target for Heads and might be the time to think how you want to support multiple targets.
This isn't a finished PR and the point is to request comments. Below are some suggestions which you might want to consider.
I haven't tried to enable all modules. I think this is a typical config with some things disabled. I'm not sure which are necessary. Might enable and fix build of other things. At the moment at least 31 out of 48 available modules build fine.
Building for power produces
zImage
instead ofbzImage
. The latter doesn't work (no rule to make bzImage
). Image name is configurable now.For coreboot, I specifed
-b
incoreboot_repo
to switch to the branch. Might want to improve this.For now I hoisted everything to board configuration via
CONFIG_<MODULE>_TARGET
variables and I don't really like how it looks (naming is very inconsistent). Maybe you want to have a genericCONFIG_TARGET
, which will then be translated within each module and fail for unknown targets? This might ease adding new targets in the future thanks to more obvious errors.