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

(BRICKS T430 because OPTION_TABLE) WiP: build xx30 boards against coreboot 4.13 #944

Closed
wants to merge 1 commit into from

Conversation

tlaurion
Copy link
Collaborator

@tlaurion tlaurion commented Dec 30, 2020

Note that option table is known to cause problem (which is included in t430 coreboot config and differentiates x230 from t430 boards as of now).

EDIT: https://mail.coreboot.org/hyperkitty/list/[email protected]/message/ZRSPTKT4HSAJYEZXFM5EZ7MFOPYJQ7R2/

@tlaurion
Copy link
Collaborator Author

tlaurion commented Dec 30, 2020

Seems like the fix is pretty simple and could be patched if required by putting additional patch file under coreboot patches/coreboot-4.13/

Available here: https://review.coreboot.org/c/coreboot/+/48407/1/src/Kconfig

t430 (xx30): @flawedworld @Thrilleratplay @alexmaloteaux @nickfrostatx @lsafd @bwachter @TrapAcid, your input would be welcome, while a risk of needing external flashing seems to be a possibility

@tlaurion
Copy link
Collaborator Author

@tlaurion
Copy link
Collaborator Author

tlaurion commented Dec 30, 2020

t430 boards build fails without patch. Adding it
False. Was linked to board name change. Now it builds, but a brick is sure to result of flashing on T430 because OPTION_TABLE.

@MrChromebox
Copy link
Contributor

@tlaurion looks like that change you linked was abandoned in favor of https://review.coreboot.org/c/coreboot/+/48429

@tlaurion
Copy link
Collaborator Author

tlaurion commented Jan 6, 2021

@MrChromebox which was then reverted... Will check it. Build error above was due to a board naming change of T430. But Option Table will fail booting for sure on T430 as of now without proper patchset.

Edit: https://github.com/coreboot/coreboot/search?q=Skip+mcache+in+post-RAM+stages+before+CBMEM+is+online&type=commits

@tlaurion tlaurion changed the title WiP: build xx30 boards against coreboot 4.13 (BRICKS T430) WiP: build xx30 boards against coreboot 4.13 Jan 6, 2021
@tlaurion tlaurion changed the title (BRICKS T430) WiP: build xx30 boards against coreboot 4.13 (BRICKS T430 because OPTION_TABLE) WiP: build xx30 boards against coreboot 4.13 Jan 6, 2021
@tlaurion
Copy link
Collaborator Author

tlaurion commented Jan 6, 2021

Not sure of the cherry-picking strategy to use here.

  • Take all merged commits since 4.13 related to src/lib/cbfs.c?
    @MrChromebox ?

@MrChromebox
Copy link
Contributor

@tlaurion if the fix in CB:48407 is sufficient, then happy to use that since it doesn't affect anything on my end, and it's clean/simple

@tlaurion
Copy link
Collaborator Author

tlaurion commented Jan 7, 2021

@MrChromebox I do not own a t430 either as of now. Will continue with #949.

@flawedworld @Thrilleratplay @alexmaloteaux @nickfrostatx @lsafd @bwachter: Help into bringing functional patchset to boot the t430 under coreboot 4.13 with OPTION_TABLE, welcome. Else, the solution is to bring yet another t430 board, without OPTION_TABLE being specified under coreboot config. (I don't remember from memory why exactly that was put there and by whom and too lazy to dig into that. People caring about OPTION_TABLE, stand up.).

@tlaurion
Copy link
Collaborator Author

tlaurion commented Jan 29, 2021

Seems like option table is not creating prejudice for other Chromebook board under 4.13

@tlaurion
Copy link
Collaborator Author

@flawedworld @Thrilleratplay @alexmaloteaux @nickfrostatx @lsafd @bwachter : Is there anyone adventurous enough to confirm that t430-maximized builds are not bricking their system with coreboot 4.13 here?

The OPTION_TABLE problem in coreboot 4.13 seems to be related only to vboot+measured boot, where this was not bricking parrot board, tested here

...So if it works, then x230 and t430 being based on coreboot 4.13 could be merged.

Build happening here: https://circleci.com/gh/tlaurion/heads/764?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link where artifacts will have the roms downloadable from there.

@tlaurion
Copy link
Collaborator Author

tlaurion commented Jan 29, 2021

Who needed option table already from yous?
If nobody can reply, I would vote to simply remove it from coreboot in all boards, the ones needing it to manually put things there at compilation time could still put it back in.

@tlaurion
Copy link
Collaborator Author

Who needed option table already from yous?
If nobody can reply, I would vote to simply remove it from coreboot in all boards, the ones needing it to manually put things there at compilation time could still put it back in.

@flawedworld @Thrilleratplay @alexmaloteaux @nickfrostatx @lsafd @bwachter that question was for you guys! :)

@tlaurion
Copy link
Collaborator Author

@jans23? @flawedworld @Thrilleratplay @alexmaloteaux @nickfrostatx @lsafd @bwachter ?
Feedback? Something?

I will remind you guys I do not own the board, and you guys are defined as board owners under #692 for testing! Thanks!

@alexmaloteaux
Copy link
Contributor

i can test it this week end but if i remember correctly , it was not building last time i checked without applying #709 (comment) from #709

it was 6 month ago so i will try again and refresh on the actual state.

@daringer
Copy link
Collaborator

I will also see to do some tests on the t430 and report back @tlaurion

@tlaurion
Copy link
Collaborator Author

tlaurion commented May 13, 2021

@daringer @alexmaloteaux This needs to be rebased, but my question here is basically if that bricks or boot. Then why OPTION_TABLE is there in coreboot config as opposed to x230 board config (I can't recall why. Was it something about users locally modifying which primary display/gfx card is by default?)

EDIT: @alexmaloteaux #709 (comment) was a VBOOT dependency for measured boot under coreboot 4.12, where measured boot was seperated (VBOOT not required for measured boot) under 4.13.

The expected results should look like #966 (comment) and #966 (comment) in terms of PCRs populated

@daringer
Copy link
Collaborator

is there a reason why the coreboot version is only bumped for the -maximized board(s)?

and for OPTION_TABLE, also don't see a reason why it is there.

@tlaurion
Copy link
Collaborator Author

tlaurion commented May 14, 2021

is there a reason why the coreboot version is only bumped for the -maximized board(s)?

Not for the moment, no.

But we will need to be cautious from this point; not the same features set will be available for maximized boards vs non maximized boards.
For example, if my memory is good, cryptsetup1 -> cryptsetup2 will not fit under non-maximized boards, nkstorecli, etc, ethernet driver, etc.

This is where specialized board configurations will appear, by lack of available BIOS region space (xx30: 7mb, xx30-maximized: 11.5mb) and where users will have choices to make for desired features set.

and for OPTION_TABLE, also don't see a reason why it is there.

No clue.

@tlaurion
Copy link
Collaborator Author

@daringer

and for OPTION_TABLE, also don't see a reason why it is there.

#274 (comment)
Slack excerpt:


Sven Semmler
Heads runs great now on my T430 but it required some modification (OPTIONS_TABLE) and back-porting a small patch to get rid of the mce warnings. How do I feed this back into the project? The patch will flow in automatically with a newer CoreBoot version, but the T430 specific configuration 'gfx_uma_size=224M' in cmos.default and 'select USE_OPTION_TABLE' and 'select STATIC_OPTION_TABLE' in Kconfig ... what about those?
Sat, Jun 19 2021
avph changed their display name to avph (Arthur Heymans, 9elements).
avph (Arthur Heymans, 9elements)
Sven Semmler why do you need that specific gfx_uma_size config?
Sven Semmler
avph (Arthur Heymans, 9elements) I am driving a 3840x2160 (4K) external display and have replaced the internal one with a 1080p display. With or without the external display attached, when booting I saw graphical fragments over the text at the top of the screen (looked like random data in the video memory). Also when the OS was running I had strange issues that looked like random memory corruption. https://github.com/QubesOS/qubes-issues/issues/6227 After enabling gfx_uma_size=224M both effects are gone.

Restore Backup failings · Issue #6227 · QubesOS/qubes-issues - GitHub
Qubes OS version R4.0.3 & R4.0.4rc1 Affected component(s) or functionality Restore Backup Brief summary failed to decrypt /var/tmp/restorej3aq3bth/vm37/root.img.001.enc:b'scrypt:Passphrase ...
avph (Arthur Heymans, 9elements)
So coreboot typically places the framebuffer inside the preallocated memory, which could be too small. The linux i915 drm driver does not have this limitation afaik. I have no idea how qubes handles framebuffers.
Sven Semmler
I can't say (because I don't know the root cause) whether the OPTIONS_TABLE by itself or the gfx_uma_size made the difference. What I can tell you is that #6227 is no longer reproducible after I made these two changes. But I see your point, there is little to suggest this is needed for every T430 out there.

@tlaurion tlaurion closed this Dec 4, 2021
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.

4 participants