t480s port #1966
t480s port #1966
Conversation
|
Ah, just realized I'll have to also update the tb.bin too. I manually created this from lenovo's binaries and they had a different resource/URL for the T480s thunderbold firmware, so best not to risk someone bricking their board with the T480 version. Edit: I just realized my commits got unsigned when I retroactively signoff'ed the commits. I will fix later. |
Confused about this. I thought the TB was common between the two boards. As part of testing, you must test tb flasshing. Flashing the t480 tb to observe if only slow charging (bad for t480s) or if fast charging works (good also for t480s), otherwise reverting to stock tb from a backup you took prior of flashing t480 and investing time to bring a distinct t480s tb.bin under xx80 blobs dir. Comparing the t480s/t480 tb.bin from libreboot and exposing results would also help.
Commits are "signed-off" by GitHub not from command line git. Can you sign those ( Also if you could squash the last two commits together so that there is no "CircleCI fix" commit, that would be awesome. You can also search t480 issues/pr and tag t480s users that were interested into the t480s. Once tagged and those users accept to be added into https://github.com/linuxboot/heads/blob/master/BOARD_TESTERS.md, you must also add yoruself there along other responders. Once all that is done, this PR will be mergeable and board testers (including yourself) accountable for testing promptly when there is coreboot/linux/platform specific changes needing to be tested in distinct PRs. Thanks for your contribution @thickfont ! Really much appreciated! |
Apologies, I wasn't clear with my tb.bin comment. I did flash my T480s board with updated thunderbolt firmware directly from lenovo's website using Libreboot's method as documented here: https://libreboot.org/docs/install/t480.html#manual-tbt.bin-extraction-optional I have not flashed the T480 thunderbolt firmware and see no reason to risk doing so if lenovo offers an updated T480s version anyway that we can use. However, I forgot to include my blob in my commit (or the steps to reproduce it similarly to how the T480 one is currently produced). I will have to fix this. I didn't actually test the slow/fast charging after updating it. I'm not sure how to test that beyond manually monitoring the charging speeds, but I'll look into it to see if the updated firmware actually does work.
Appreciate the feedback. Will get to fixing all of the above. (I'm a git newbie :) ) |
|
My point is still valid here: if you are to provide a tb.bin, you will have to test it :) |
|
@thickfont great to see it worked! |
|
@thickfont in case you did not do it yet, it would be really helpful for other less experienced user to have at least one picture of disassembled t480s board on the heads-wiki flashing guide. In my view, it can really reduce the stress level if the location of the SPI chips is different. |
|
@thickfont it looks like the build failed… |
No its #1965 |
|
I added the t480s tb.bin to the blob download script. Both t480 and t480s currently share the tb.bin file name in the blobs/xx80 directory. So if you build t480 board, the tb.bin file will be for t480. Whereas if you build t480s board, the tb.bin file will be for the t480s. In other words both tb.bin files currently can't co-exist in the directory if someone built both boards. Both file hashes co-exist in hashes.txt too (not ideal, but it works if you know to expect an error if you manually run "sha256sum -c hashes.txt"). I would have named the tb.bin files "t480_tb.bin" and "t480s_tb.bin", but that would require modifications to the targets/xx80_me_blobs.mk file (as it references the blobs/xx80/tb.bin file directly). I don't understand the formatting of this file to confidently modify it for both possible tb.bin file names. EDIT:
You can see the first line is the output from this line in the script: Therefore, $sha256_hash is null which is actually $TB_BIN_HASH which is set in my new if/else statement which assumes the local BOARD environment variable is set. $TB_BIN_HASH will only be null if the BOARD environment variable is not set. Is this variable not set in the context of CircleCI's execution of this script? If so, how else can can I determine which board is being built within this script? Is there better logic for this? The second line of the above logs (missing tb.bin) is also explained by the same missing BOARD environment variable. |
|
@akunterkontrolle @mblanqui @kjkent Are any of you guys interested in being official board testers for the T480s? I found you guys displaying varying levels of interest in a t480s port in other threads. Cheers! Ref: https://github.com/linuxboot/heads/blob/master/BOARD_TESTERS.md EDIT: Don't flash anything yet though! I haven't flashed roms from circleci's yet, so I can't guarantee they work even though they should (hashes are different from my locally built and tested roms which I think is normal until the PR is merged?). |
|
@thickfont sure am! Happy to contribute. |
@thickfont the roms should match, where local build artifats differences for same commit are typically linked to non-clean cpio that are not rebuilt. You should arrive to the same CircleCI built artifacts if local is clean, by doing the following if final rom hashes are different This should be fixed with #1961 that should be merged today (which seems to have fixed initrd.cpio-xz rebuild, but if not, another issue should be opened) Also note that CircleCI failing to build has been merged in master just now under #1967. You should be able to merge master into this PR by doing
|
@thickfont as said in previous comment, tb rom differences, and if such difference needed, should be validated and tested prior of me investing time helping for the port here with commits. As previously asked: have you figured out under libreboot why those tb.bin are the different and if needed as requested? if the t480 tb file can be used on t480s then no need for the following. But if needed, here is what is needed: Also, I saw you partially added differential tb bins but reuse same tb.bin instead of duplicated in blob script. The checksum should be unique for each file; no tb.bin should be shared if required to be different, this can lead to bricks and CI as confirmed by CI, logic is wrong: the blob script should should download and reconstruct both on a CircleCI prep_step; therefore i'm not sure your conditional BOARD logic is the right thing to do under a shared shared blob script; either you duplicate the blob script into two t480/t480s, split the scripts into shared degard+me/ separate tb480 + separate tb480s or have a single script that doenloads and reconstructs blobs needed for both t480s and t480. You should figure out which option is better for you and others to understand and maintain (I think splitting blobs download script into t480/t480s and same for target should make things clearer to yyou and easier to reuse existing code)so that they require and point to individual tb files, and for those to be created, depend on individual blob script (that seems to be where you got confused with @gaspar-ilom code which uses variables to point ot checksum and files you didn't completely duplicate in your previous attempt). Current issue with blob script issue and non-duplicated target/xx80: for Makefile logic under target/xx80, same thing: duplicate into t480 and t480s and rename to t480s_tb.bin and t480_tb.bin as your duplicated in xx80/blob download +pad + mv into xx80 blob dir and you should be able to resolve circeci no need to duplicate config/linux-t480s.config here: they do not have a single difference. Just have the board config for t480s point to t480 linux config Time spent currently on t480s review: 2h total. Trying to mentor and code review instead of doing the changes this time. Might take longer but more chances of others to do ports and succeed. We can see #1658 having been dropped as an effort after a lot of work. Sorry to not invest more time for other people's desired board without doing paid consultation services anymore; revising approach, let me know what I can do better for others to be able to port things better and help co-maintain Heads on free tier. |
|
Thanks for the review @tlaurion, exactly the kind of feedback I was hoping for. Much appreciated. Will try to fix most if not all of this today. I reviewed the libreboot docs and their tb.bin implementation. They are using different tb.bins for each board, and they also say "Please make sure to flash the correct one for your board". Goes without saying that The tb.bins have different hashes too. I see no reason to test the t480 tb.bin on my only remaining t480s board unless you can guarantee it won't brick the board... |
thickfont
force-pushed
the
poc_t480s
branch
3 times, most recently
from
964a936 to
3e8a180
Compare
Signed-off-by: thickfont <[email protected]>
thickfont
force-pushed
the
poc_t480s
branch
2 times, most recently
from
0e7546e to
afeb6ea
Compare
This is really problematic. Before merging this PR, at least one of the BOARD_TESTERS.md updated file for t480s needs to flash that t480s_tb.bin, ideally the one porting the t480s. No, I cannot guarantee that it won't brick your USB port's fast charging capabilities, as libreboot produced documentation nad guide to reproduce those blobs, and t480 board challenged that documentation prior of creating a flashing page with the facts. I'm sorry @thickfont but if you are weary on flashing that blob, but that untested blob comes on your PR, which PR content is supposed to be tested working at the best of your knowledge and to the best of my review: we have a problem, since you basically say here that you are not willing to test/report/collaborate with libreboot on things not fcuntional, and as I stated numerous times before, after having spent more than 30h on the t480 and challenging Leah (with all the drama that came with it) and with the t480/t480s still not merged under coreboot, the current PR will not be merged. We are mixing things here. I am willing to give advice/mentoring/share knowledge to those who want to have Heads on the platforms they port to Heads. I am not willing dealing with support requests technical board owners are not willing to test nor fix themselves nor challenge upstream (here libreboot until coreboot) since I am not paid for this work either. TLDR: if you are not willing to test libreboot instructions yourself for the t480s tb bins CircleCI produced, nor any other technical enough board owners (candidated @akunterkontrolle @mblanqui @kjkent) stand up to the task of being able to maintain/test this board/participate upstream under libreboot/coreboot here, you are basically expecting me to deal with consequences of other people flashing things that should be tested but weren't and might brick their boards. It would not be ethical to merge this. If this is what you are aiming to do, I would gladly ask you to do another commit which will rename the t480s as UNTESTED (there are helpers under global Makefile to facilitate this), as until one more technical board owner stands up as being the one that will maintain this board. My job as a maintainer of Heads is to make sure that changes needed across all supported boards are tested per their board testers in time for possible linked regression for future PR. I'm sorry if there was still a misunderstanding, but the goal of the Heads project is to have supported boards and a community to help each other support those board. Here, libreboot is upstream until merged under coreboot at https://review.coreboot.org/c/coreboot/+/83274, so under libreboot community channels. There is no way anything merged under Heads becomes Heads responsibility because other people in the accountability chain fails their responsibilities. Yours here is to flash all build artifacts and confirm no other regressions happen in terms of coreboot+libreboot so that people that would arrive to me would be only for things Head srelated. There is no way I extend my role outside of that. I have no time for that, nor own a t480s. I hope we are clear and there is nothing personal here. I understand you are excited by the t480s being WiP under coreboot at https://review.coreboot.org/c/coreboot/+/83274 and supported by libreboot. But reality here is that it is not supported by coreboot today. So no. I cannot guarantee you that flashing that tb.bin will not make that controller not work again. But if we stay logical, and you get a backup before flashing, you should not be scared of flashing nor reverting things, otherwise there is no technical enough board owners behind this port for the t480s. Again, nothing personal here. But if its the case, we need someone else to stand up that woun't be scared of flashing, causing bricks, and reverting to backups. Otherwise this PR won't get merged. |
Awesome progress with the other commits. Hope my previous comment facts reached you and what needs to be done prior of merging this pr which now builds from CircleCI and should produce same roms loally and from CircleCI. Please confirm and let me know if you can be that responsive technical board owher willing to risk a brick and unbrick/be on the libreboot channel until https://review.coreboot.org/c/coreboot/+/83274 gets merged, otherwise this PR will bitrot until someone else show up and is willing to create SPI content backups/flash/revert to backup any future PR related to the t480s, as for any supported boards under Heads. |
|
Updated OP TODOs |
|
I'm not overly concerned about bricking my t480s or its thunderbolt SPI and can flash and report back once it's decided what's happening with this PR. I have no thunderbolt devices, but can at least report on the port's USB-C and charging functionality. I can confirm that the tb.bin produced by LBMK for the t480s works in relation to the above, offering a second easy source for a clean rom (the first being a self-made backup) to revert to in case of issues. @thickfont @tlaurion thanks for all the work you're both doing on this! I hope the PR can be merged, as it would bring these modern and capable ThinkPads to more people looking for a secure boot chain. I'm not yet sufficiently familiar enough with the project, but that will change and I am interested in contributing when able. |
@kjkent : I'm just saying again that coreboot side is Work In Progress (WiP), unmerged work coreboot upstream, with known issues that have nothing to do with Heads, and are related to coreboot fork, so people here and from t480 need to be confortable with those and help upstream move forward, support the coreboot developers. Heads is just using the coreboot base and adding Heads related features. See https://review.coreboot.org/c/coreboot/+/83274 It will be merged if reported as working as expected, properly documented and with people ready to test when needed, as for any other boards supported under Heads, with the t480/t480s being the current exception to the rule of being supported properly either in a fork or coreboot upstream. Libreboot actually uses the patch that was developed by @gaspar-ilom on top of coreboot WiP; nothing was fixed further for now. Let's hope t480/t480s get merged upstream coreboot soon, otherwise I will not take those things farther since I have no personal interests neither in t480 nor t480s and I have already contributed with my time and energy enough for the t480, time for someone else to care for things further with libreboot/coreboot. Having support under Heads as of today is not sustainable and won't last:
Do libreboot tb and heads produced tb for t480s have the same hash? If so you confirm here that you flashed that rom, @kjkent and would be willing to use a cell phone that fast charges and reports fast charging to report on that t480s_tb.bin being flashed on your t480s externally.
Excellent! |
|
From my previous posts:
So, just to recap. I have flashed my T480s with new updated thunderbolt firmware. I had just forgot to include it in my initial PR (since I manually built the tb.bin not using any scripts at the time). I have since included this t480s tb.bin in the PR using the blob scripts. NOTE: The t480s tb.bin that I flashed and now included in this PR, is the same tb.bin that libreboot builds. It has the same hash. I did say I haven't tested it yet because I didn't know how... But now you suggest using a phone, so I will and confirm. When I said:
I am literally talking about flashing the t480 tb.bin on a t480s. I have not done this for reasons previously outlined. Does this clarify your confusion? @tlaurion |
Yes, and thanks for clarifying. From my previous reading, I understood you upgraded tb firmware from other means and weren't willing to flash externally the t480s tb firmware built here, having same hash of libreboot's. So I guess the only thing missing, to be ticked in OP as well, is a clone of the T480 page for t480s with disassembly picture. As you see fit, either referring to t480 page for everything else then the disassembly and different placement of tb chip, or a clone of it and board tester file being modified per this PR as well. I would then need a second, referred board tester to confirm flashing roms from this PR working for him too and then can merge both this PR and the heads-wiki one at the same time. Sorry if I misread @thickfont, and thanks for this contribution for all that will benefit from it. I'd you have any comments or edits to do on the porting guide, that would also be welcome. |
|
No worries on the misunderstanding, totally understandable with the t480/t480s naming similarities. I have some comments for the porting guide I will share later after I collect my thoughts. I will happily contribute to the wiki too, will just need some more time. Changing topics back to this:
Something odd is happening when I build locally in my repo locally using: The resulting .rom is named: Which is missing the commit hash info usually appended to the end of the file name. I suspect this is why my hashes are different than what is being built in circleci? Since the roms embed the commit hash data and my locally build roms are missing them, this should explain the differing resultant rom files and hashes, right? Any ideas why mine are missing the commit data? Have you seen this behavior before? EDIT: For additional context, when I initially made my edits directly in my local clone of the linuxboot/heads repo, the roms built there did correctly have commit data appended to the file names. It is only since I transitioned to my own repo (e.g. thickfont/heads) for the PR that the roms file names have been missing the data. |
I'll build & diff them later today, and can test flash the heads binary too if I get chance this evening. Will report back. Edit: I just read up and saw that @thickfont has already flashed and confirmed the tb binary is identical to that of LBMK -- my apologies! |
@thickfont Someone had that issue before, but cannot find the solution easily since it was not filed in an issue. Global Makefile extracts pieces used for filename really early at: Lines 1 to 13 in 13f55f4 And are then reused later on to construct filename: Lines 99 to 104 in 13f55f4 So in your case, CB_OUTPUT_FILE (used as rom base filename) is wrong. Can you do from local build env what is done to construct EDIT
Not sure how you did this.
Can you detail your process differences maybe? |
|
I just built locally using your steps and it did fix my issue of missing the commit data at the end of the filename. But I'm still getting a different hash. Here were my exact steps: I decided to compare the hashes.txt file between circleci and local. After sorting both hashes.txt with (they were both in different filename orders): I diff'ed the resulting sorted files and got this result: So what gives? Is CircleCi not cleaning its cpio? Given that I built locally in a fresh git clone'ed repo from scratch, surely a clean rom was built? |
|
@thickfont I also get 911ff63584bd4f3115efe4fe44aacec29dca47bc0e32f1d7d546ddacf266eb02 for data.cpio (and therefore same initrd.cpio.xz and final rom hashes), for all boards not excluding kbd and keyboard layout keymaps introduced in #1961. data.cpio being into initrd.cpio.xz being into final rom: this explains why the 3 files differ for you since data.cpio is different. hashes.txt reports: cpio, then content. Which files are not in the same order? What I use for reproducibility check, downloading hashes.txt from t480s-maximized board's CircleCI artifacts tab: Which confirms that hashes.txt data.cpio content is all equal on my local build, and circleci: More notes under linuxboot/heads-wiki#70, but as of now there needs to be something different in the data.cpio for it to be different otherwise i'm not understanding what is happening here: the cpio should be constructed reproducibly unless I did something bad when introducing keyboard keymaps #1961. Zip that data.cpio and post it here? |
|
Can confirm I get the same output with your one-liner: Hashes: |
|
Just built this PR with the goal of diffing to confirm and then flashing. The ThunderBolt firmware roms (ie., LBMK's tb.bin to this PR's The comment above these |
Valid concern. However, I can confirm the T480 me blob works on the T480s. As mentioned here:
However, I don't know what "first link" they are referring to as a source (none of the links say so?). I am willing to switch it over to a T480s me blob, though I don't know if it matters at all? With the current differing hash issues, I wouldn't flash anything yet. Flash at your own risk. Once hashes match, I'd prefer to be the first to risk the flash (not that I haven't already gotten it working, but I can't guarantee it will work unless I flash the exact same rom as circleci) Btw, since you built the board, do your hashes match those in circleci? |
I understood this as meaning that the executable linked to (i.e., https://dl.dell.com/FOLDER04573471M/1/Inspiron_5468_1.3.0.exe) contains an ME that can be deployed to both devices, without inferring that the processing it must undergo via
That was originally written in f75ddb8 by @gaspar-ilom during the original port of the t480 -- I'm not sure either but
Comparing the hex diffs of the two relevant directories in Out of curiosity, I'll check out how LBMK approaches this.
Caution understood, thanks :) And yep, my hashes match for the final rom and thunderbolt binary, but the keymaps output differs (appears to be different keymap set built between local and CircleCI?), and |
|
@kjkent Good points. I also decided to look into it too. The current "download_clean_deguard_me_pad_tb.sh" script git checkouts a commit of deguard that does support thinkpad t480s in the data/delta directory. So, it should just be a matter of changing the ME_delta variable to t480s and it should work just like lbmk. I see no reason to not do this, we might get ahead of some unforeseeable bugs by doing so. Added to my todo list.
I don't follow you here, how are you getting the same rom with different keymaps used in your local build? What do you mean when you say you different keymaps? I just built t480s-hotp-maximized and t480s-maximized on a second piece of hardware and also got the same different hash as my previous local builds. EDIT: |
…selected and separate blobs Signed-off-by: thickfont <[email protected]>
Signed-off-by: thickfont <[email protected]>
Signed-off-by: thickfont <[email protected]>
Signed-off-by: thickfont <[email protected]>
Signed-off-by: thickfont <[email protected]>
|
I updated the blob script to use the t480s delta for the deguard of the ME. The resulting me.bin now is identical to the one created by lbmk. During my rebase, I left out the git merge to the latest keymap updates to see if the roms are reproducible. I can confirm they are reproducible (local vs circleci) for me without the merge. So it seems the keymap update might be the culprit? Lastly, I finally checked the thunderbolt fast charging and can confirm fast charging works. I updated the todo list to reflect this. |
I opened #1969 to track this issue. For the moment, without investigating further, I do not understand why some local builds differ from Circleci and wasn't able to reproduce. Will check next week. |
Testing Checklist:
RPi PicoQubesOS 4.2install and rebootNitrokey 3ahardwarekeyQubesOS 4.2Happy to provide screenshots/proof at a later date.
Thanks to @tlaurion @notgivenby for their help in Matrix chat. I barely did anything for this port, the pieces were simply waiting to be put together thanks to the major efforts by the parties responsible for the T480 port.