Patching Coreboot before compiling?

[natterangell]

Hi,

I apologize as this is not technically an issue with Heads, but I'm requesting guidance here as it might be relevant for other T420 users too.

There is an unmerged patch on Coreboot Gerrit that fixes an issue with wwan cards on the T420. In short, the patch enables power on wwan if slot has SMBus connected. Without this, the card will pull the lines low, causing DRAM init to fail.

This patch allows me to use a Sierra Wireless MC7304 LTE capable card, instead of my ancient Ericsson F3507g, which is rather nice since older 2G/3G cards are more or less irrelevant these days.

I suppose the current Makefile verifies hashes for coreboot(?). So, how should I manually edit the coreboot source files when building Heads?

[tlaurion]

@natterangell you would have adapt that patch and apply it from patches/coreboot-4.13/wwan-smbus.patch or similar, which is applied on top of validated coreboot 4.13 tarball sha256sum hash.

Note that patches are applied only when the module's build dir doesn't exist, so you have to remove coreboot directory under build/x86/coreboot for that patch to be applied on clean extracted tarball.

You can test application of your patch on top of coreboot tarball extraction in another directory first. Package is under packages/arch/

This patch doesn't seem complete and implementation was questioned seeing upstream comments.

Tag me if you need more info on testing this. Upon successful PR, regression tests on other boards would be needed, since most xx20/xx30 boards are affected by this patch.

[tlaurion]

@natterangell 2dad98d just adds the upstream patch to be applied against coreboot 4.13 without further modification.

Doing so in a branch on your github, followed under your CircleCI account would result in a build starting to build all roms and see if things break at a build level.

This is happening with a blunt addition of upstream patch in a branch over my github for Heads project at:
https://app.circleci.com/pipelines/github/tlaurion/heads/1209/workflows/f7310d77-131b-440c-96ac-df2903c3f11a/jobs/10749

If the commit goes green, it means the build passed without breaking anything. Otherwise, the build having failed will show where the error happened under "Making Board" last lines of such board. Basically, in this first step, failing will probably be the result of a patch not being directly applicable, showing what lines of the patch needs to be reworked for it to be successfully applied.

[tlaurion]

@natterangell : we see it failing in the first coreboot 4.13 built board at https://app.circleci.com/pipelines/github/tlaurion/heads/1209/workflows/f7310d77-131b-440c-96ac-df2903c3f11a/jobs/10751?invite=true#step-103-55

(Idea of CircleCI and its caching system is that once a module patch changes, CircleCI will build on a clean checkout. This means that its the proper way to expose build issue and seek help if needed, since just as exposed here, we can then relate to what failed, including build logs, to make sure we are talking about the right and same things, on a clean checkout of a specific commit, trying to accomplish a limited change without having to disregard all other possible causes of failing).

So here, the patch in question is applied after having downloaded and verified that coreboot 4.13 tarball is valid, extracting it and then applying the patch that is verified per commit (implying trust into me not having modified it.)

On my side I just downloaded the patch from https://review.coreboot.org/changes/coreboot~39043/revisions/7/patch?zip, extracted it and moved it into a proper named patch file as you can see under tlaurion@2dad98d.

I then added that patch into a new branch (git checkout osresearch/master, git reset --hard, git checkout -b name_of_branch, git add patches/directory_of_module/name_of_added_patch, git commit -m "description of commit", git push tlaurion-github --force).

Then since CircleCI is watching all my github Heads branches, it started a build for that commit with CircleCI already defined boards under CircleCI config in tree and failed as expected :

Applying patch file : patches/coreboot-4.13/0081-wwan-smbus.patch 
patching file src/ec/lenovo/h8/Makefile.inc
patching file src/ec/lenovo/h8/wwan.c
Hunk #1 succeeded at 24 with fuzz 2 (offset -13 lines).
patching file src/mainboard/lenovo/t420/early_init.c
Hunk #1 succeeded at 1 with fuzz 1 (offset -15 lines).
Hunk #2 succeeded at 61 (offset -15 lines).
patching file src/mainboard/lenovo/t430/early_init.c
Hunk #1 succeeded at 2 (offset -13 lines).
Hunk #2 succeeded at 57 (offset -13 lines).
patching file src/mainboard/lenovo/t520/early_init.c
Hunk #1 succeeded at 3 (offset -15 lines).
Hunk #2 succeeded at 56 (offset -15 lines).
patching file src/mainboard/lenovo/t530/early_init.c
Hunk #1 succeeded at 3 (offset -15 lines).
Hunk #2 succeeded at 39 (offset -15 lines).
patching file src/mainboard/lenovo/x220/early_init.c
Hunk #1 FAILED at 18.
Hunk #2 succeeded at 57 (offset -17 lines).
1 out of 2 hunks FAILED -- saving rejects to file src/mainboard/lenovo/x220/early_init.c.rej
can't find file to patch at input line 175
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/src/mainboard/lenovo/x230/early_init.c b/src/mainboard/lenovo/x230/early_init.c
|index 70240a7..54513fe 100644
|--- a/src/mainboard/lenovo/x230/early_init.c
|+++ b/src/mainboard/lenovo/x230/early_init.c
--------------------------
File to patch: EOF
Skip this patch? [y] EOF
Skipping patch.
2 out of 2 hunks ignored
make: *** [Makefile:410: /root/project/build/x86/coreboot-4.13/.canary] Error 1

So the patch needs to be reworked to be applicable on top of 4.13 at tlaurion@2dad98d#diff-082d6300fec0627b1230169d1c34c89f7695e060591ba95bad304a2daaf6084cR175

Where a local inspection confirms that there is no existing build/x86/coreboot-4.13/src/mainboard/lenovo/x230/early_init.c file to be patched, explaining why patching fails and needs the patch to be reworked and retested iteratively.

[natterangell]

Wow, thank you for that! 😊

yes, I’m aware the patch has issues, but it might be useful for my machine still.

It seems to fail for the x220, and succeed patching early_init.c for the t420. I’m not even sure this issue applies to the x220, it certainly doesn’t for the x230.

I can do a manual test as outlined in your first reply.

[tlaurion]

@natterangell On coreboot matters, its always the same thing. When things are worked on downstream, it doesn't make things merged upstream. Collaboration should happen upstream here. Unfortunately, Heads is still based on 4.13, so there are patches that will fail to apply against 4.17+ (coreboot 4.18 to be released being master on coreboot as of now).

Whatever your outcome here, adding that as being successful, commenting upstream, (and removing x220 and x230 downstream in a branch of yours linking to it) and adding feedback under https://review.coreboot.org/c/coreboot/+/39043 would help everybody. Otherwise, that wwan SMBus patch is bitrotting there (already having conflict against coreboot master in its current state).

It seems that coreboot 4.13's x230 codebase doesn't have early_init file (which could be removed in patch) while resolving issue with t420. It also shows some discrepancies across boards under 4.13 (different codepaths on that level for 4.13).

After being successful in your tests, you could then report success upstream for your board, which would ping the developers and make this patch merged upstream faster. That would result into that wwan SMBus patch being merged under 4.18 or sooner then later, and have Heads not need any patch when we do a version bump to coreboot 4.18, hopefully sooner than later :)

[tlaurion]

Massively edited #1221 (comment) so people can replicate and test CircleCI builds fast against OP's future related questions related to : "Patching Coreboot before compiling".

Outside of commenting this ticket, doing the process to actually create a discussable outcome took me literally 30 seconds with everything already being setuped on my side. (local git branch, remote branches setuped, CircleCI account and watching github repos from there).

And for everybody to be able to see the result of a failed applied patch to entice collaboration for people having the same feature goal to be able to work locally on a fix and discuss on possible solutions, experiement and propose a working solution that don't break anything (on a build level). The next step of this is to check from the commit what platform could break with actual proposed changes, and be able to to some regression testing prior of the change to land in master after a pull request is poroposed, reviewed and finally merged after proper regression testing per board owners for changes that could affect them.

[natterangell]

Thanks again! I’ll be sure to report back upstream once I’ve tested. If I can find the time I’ll prepare a branch for it. It’s a learning curve for me.

[tlaurion]

@natterangell cloning/branching Heads deserves a new wiki entree/bonifying under https://osresearch.net/Development

Command line is always better for that.... Agreed there is a learning curve associated to that though.

This could be used as a draft to be added there.

1. Create a fork under Github at https://github.com/osresearch/heads/fork so that changes can be localized and branches submitted on personal fork
2. Have github know your ssh public key so you can upload back your changes to github in your fork https://medium.datadriveninvestor.com/how-to-clone-a-github-repository-using-ssh-for-windows-linux-macos-78ad9a3959e?gi=a48768b328cd
3. Have GitHub know your pulic key so that you can sign your commits (this is important so that commits can be trusted and have accountability) https://www.freecodecamp.org/news/git-ssh-how-to/
4. Clone Heads (osresearch/master : will be origin) : git clone https://github.com/osresearch/heads
5. Make sure latest version of osresearch/heads git fetch origin
6. Add your fork's remote to be useable with ssh: git remote add GITHUB_USERNAME [email protected]:GITHUB_USERNAME/heads.git
7. git fetch GITHUB_USERNAME
8. I always make sure I'm at the right latest commit against master prior of changing branch : git checkout GITHUB_NAME/master
9. git reset --hard
10. Change to a branch you want to track your changes: git checkout -b NAME_OF_CHANGESET
11. Do limited changes to accomplish a goal (a commit should contain changes linked to accomplish one single thing at a time, and a branch should contain the smallest number of commits needed to accomplish that goal. Think iterative, your branch might be asked to be reworked to seperate your changes across multiple pull requests (different goals) if too many commits are present. A good pull request (your branch pushed against master) should contain maximum 4 commits unless really justifiable.
12. git add FILE1 FILE2 FILE3
13. Get information on current changes: git status
14. git commit / git commit -m "goal: adding function xyz"
15. continue doing your changes
16. Add your changed and do commits as above
17. When done, git push GITHUB_USERNAME, or force pushing changes if you modified history of commits git push GITHUB_USERNAME --force

On step 16 above, when it is asked of you to modify your pull request, you might have to dig a little bit more the internals of git.

• git commit -amend will permit you to modify your last commit
• Interactive rebasing will permit you to go back in your commit history to squash changes, amend commits etc: https://thoughtbot.com/blog/git-interactive-rebase-squash-amend-rewriting-history

Otherwise, there is plenty of GUI frontends to git out there that you might want to investigate if command line git is giving you nausea. Unfortunetaly, I do not use them so I cannot really recommend any of them. I would go against using the web based VisualStudio frontend proposed by github, since even though it might seem attractive since proposed by Github, amending/rebasing and pushing changes results in a lot of back and forth and many pull requests being opened/closed in the past. Doing changes locally and pusshing them is definitely more interesting, both for your learning and collaborating to other projects and for maintainership of this perticular project.

CircleCI:

1. Open account on CircleCI
2. Subscribe on your github project.
3. That's it. When you will push commits to your fork, CircleCI will build automatically and will inform your branch of success/failed commits. that should probably be another entree.

Will edit this post later on following feedback, and will use that entree here to open another issue in wiki documentation.

[tlaurion]

@natterangell can this be closed?

[natterangell]

Yes! Thanks!

[tlaurion]

@natterangell maybe you want to rename the issue?

Or give some more input on if the patch was merged upstream? In which version, etc?

[natterangell]

I haven't gotten around to suggesting a patch upstream yet, I'm a little short on time these days. I hope to get to it before the end of February.

[tlaurion]

Documentation issue opened on heads-wiki. Closing here, feel free to reopen if again relevant.