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

T420 initial support + X220 FBWhiptail Support #578

Merged
merged 33 commits into from
Feb 19, 2020

Conversation

snmcmillan
Copy link
Contributor

Separating my original PR #539.
This branch introduces T420 support (complete with FBWhiptail support) and modifications to the X220 to support FBWhiptail.

No known new regressions are present, and this does not fix the 50 second delay that is currently present.

snmcmillan and others added 21 commits March 20, 2019 09:14
…estarting, or during S3 resume. If they actually bring functionality to builds, replace the patch with something that is tested.
…ng up, restarting, or during S3 resume. If they actually bring functionality to builds, replace the patch with something that is tested."

This reverts commit 41e5301.
adds the chip option, needed for X220 to actually use this.
Forgot to disable TPM logging
Copy link
Contributor

@merge merge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO this should be a seperate PR!

Copy link
Contributor

@merge merge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the commit message should better read "blobs/t420: remove gitignore"

# Configuration for a x220 running Qubes and other OS
# The Linux configuration is close enough to the x230
export CONFIG_COREBOOT=y
CONFIG_COREBOOT_CONFIG=config/coreboot-x220.config
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is that correct?

# The Linux configuration is close enough to the x230
export CONFIG_COREBOOT=y
CONFIG_COREBOOT_CONFIG=config/coreboot-x220.config
CONFIG_LINUX_CONFIG=config/linux-x230.config
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if correct, we should add a comment, why, I think

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is correct. If the already existing X220 is close enough to X230, and T420 is close enough to X220, T420 is close enough to X230.

Tested on my T420 as working.

Copy link
Contributor

@merge merge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be squashed into the previous change that adds t420.config

export CONFIG_COREBOOT=y
CONFIG_COREBOOT_CONFIG=config/coreboot-t420.config
CONFIG_COREBOOT_CONFIG=config/coreboot-T420.config
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

really uppercase T?

Copy link
Contributor

@merge merge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not do 2 commit when changing 2 board configs?

Copy link
Contributor

@merge merge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to review this seperately and this has to be a different PR. interesting...

This reverts commit 1e705eb.

Reverting that way the new method to obtain flash definitions can be used (seen in osresearch/linuxboot#592)
@snmcmillan
Copy link
Contributor Author

This branch's flash.sh changes are reverted to conform with #592's way of supplying flash definitions.

@tlaurion
Copy link
Collaborator

tlaurion commented Jan 2, 2020

@SebastianMcMillan Have you had a chance to test including ifd and reduce me and changing CBFS size in coreboot config and see if the 60 seconds delay linked to ME disappeared?

@snmcmillan
Copy link
Contributor Author

snmcmillan commented Jan 2, 2020 via email

@techge
Copy link
Contributor

techge commented Jan 3, 2020

@SebastianMcMillan Have you had a chance to test including ifd and reduce me and changing CBFS size in coreboot config and see if the 60 seconds delay linked to ME disappeared?

As mentioned in #539 I followed the guide and truncated the ME, still the boot delay is valid.

@techge
Copy link
Contributor

techge commented Jan 3, 2020

Commenting CONFIG_MEASURED_BOOT here stops the delay. So that means it is a TPM issue, isn't it?

techge added a commit to techge/heads that referenced this pull request Jan 3, 2020
@snmcmillan
Copy link
Contributor Author

I don't believe this is an ME issue. I can use the exact same IFD and ME with stock coreboot and we don't see this issue. Furthermore, using stock ME and IFD still reproduces this issue.

This issue is certainly a 4.8.1 issue. I've tested pre 4.8.1 heads on my X220 and can confirm the issue is not present. At this point, I will wait for heads to be based on 4.11+ and see if it's still an issue.

@techge
Copy link
Contributor

techge commented Jan 13, 2020

I would like to already play around a bit. So may I ask how I could switch to old coreboot in the meantime?

@techge
Copy link
Contributor

techge commented Jan 21, 2020

Okay, I used this PR as a basis for my own branch and combined with the hint from @tlaurion it actually works fine now for me!

Would @SebastianMcMillan like to separate the x220 stuff as I did and create a new PR as this is tested now, shall we wait or shall I create a PR based on the branch I mentioned above?

@snmcmillan
Copy link
Contributor Author

snmcmillan commented Jan 21, 2020 via email

@tlaurion
Copy link
Collaborator

@SebastianMcMillan ?

@snmcmillan
Copy link
Contributor Author

I apologize on the delay, my T420 is running into some difficulties, and I've been a little caught up with coursework. I am working on testing @tlaurion's suggestion on the X220 as well.

Fixed Typo for flashrom_options
Added intel_iommu=igfx_off argument
Decreased CBFS size to fix boot delay problems
Enabled compiling with any toolchain to prevent build problems
Removed check ME option since it fails to check the ME due to the build process not finding me_cleaner
@snmcmillan
Copy link
Contributor Author

T420 and X220 boot delay fixes tested and confirmed working on my end.

Fixed boot delay by decreasing CBFS size, removed dysfunctional check ME option, and enabled use of any toolchain.
@tlaurion tlaurion merged commit 21faf52 into linuxboot:master Feb 19, 2020
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