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

Add initial support for the ThinkPad T440p #1282

Merged
merged 19 commits into from
Feb 28, 2023
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 15 additions & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ commands:
command: |
ln -fs /usr/share/zoneinfo/America/New_York /etc/localtime
apt update
apt install -y build-essential zlib1g-dev uuid-dev libdigest-sha-perl libelf-dev bc bzip2 bison flex git gnupg gawk iasl m4 nasm patch python python2 python3 wget gnat cpio ccache pkg-config cmake libusb-1.0-0-dev autoconf texinfo ncurses-dev doxygen graphviz udev libudev1 libudev-dev automake libtool rsync innoextract sudo libssl-dev device-tree-compiler u-boot-tools
apt install -y build-essential zlib1g-dev uuid-dev libdigest-sha-perl libelf-dev bc bzip2 bison flex git gnupg gawk iasl m4 nasm patch python python2 python3 wget gnat cpio ccache pkg-config cmake libusb-1.0-0-dev autoconf texinfo ncurses-dev doxygen graphviz udev libudev1 libudev-dev automake libtool rsync innoextract sudo libssl-dev device-tree-compiler u-boot-tools sharutils e2fsprogs parted curl unzip
- run:
name: Make Board
command: |
Expand Down Expand Up @@ -429,6 +429,20 @@ workflows:
requires:
- x230-hotp-maximized

- build:
name: t440p-maximized
target: t440p-maximized
subcommand: ""
requires:
- x230-hotp-maximized

- build:
name: t440p-hotp-maximized
target: t440p-hotp-maximized
subcommand: ""
requires:
- x230-hotp-maximized

- build:
name: qemu-coreboot
target: qemu-coreboot
Expand Down
42 changes: 42 additions & 0 deletions blobs/haswell/obtain-mrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
#!/bin/bash

set -e

function usage() {
echo -n \
"Usage: $(basename "$0") path_to_output_directory
Obtain mrc.bin from a Haswell Chromebook firmware image.
"
}

MRC_BIN_HASH="d368ba45096a3b5490ed27014e1f9004bc363434ffdce0c368c08a89c4746722"

if [[ "${BASH_SOURCE[0]}" == "$0" ]]; then
if [[ "${1:-}" == "--help" ]]; then
usage
else
if [[ -z "${COREBOOT_DIR}" ]]; then
echo "ERROR: No COREBOOT_DIR variable defined."
exit 1
fi
rbreslow marked this conversation as resolved.
Show resolved Hide resolved

if [[ ! -f "$1/mrc.bin" ]]; then
pushd "${COREBOOT_DIR}"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If a mrc.bin already exists, don't try and download it again. Instead, skip right to the integrity check.


# https://doc.coreboot.org/northbridge/intel/haswell/mrc.bin.html#obtaining-mrc-bin
make -C util/cbfstool
cd util/chromeos
./crosfirmware.sh peppy
../cbfstool/cbfstool coreboot-*.bin extract -f mrc.bin -n mrc.bin -r RO_SECTION

popd

mv "${COREBOOT_DIR}/util/chromeos/mrc.bin" "$1/mrc.bin"
fi

if ! echo "${MRC_BIN_HASH} $1/mrc.bin" | sha256sum --check; then
echo "ERROR: SHA256 checksum for mrc.bin doesn't match."
exit 1
fi
fi
fi
53 changes: 53 additions & 0 deletions blobs/t440p/download-clean-me
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
#!/bin/bash

set -e

function usage() {
echo -n \
"Usage: $(basename "$0") path_to_output_directory
Download Intel ME firmware from Lenovo, neutralize, and shrink.
"
}

ME_BIN_HASH="b7cf4c0cf514bbf279d9fddb12c34fca5c1c23e94b000c26275369b924ab9c25"

if [[ "${BASH_SOURCE[0]}" == "$0" ]]; then
if [[ "${1:-}" == "--help" ]]; then
usage
else
if [[ -z "${COREBOOT_DIR}" ]]; then
echo "ERROR: No COREBOOT_DIR variable defined."
exit 1
fi

if [[ ! -f "$1/me.bin" ]]; then
pushd "$(mktemp -d)"

curl -O https://download.lenovo.com/pccbbs/mobiles/glrg22ww.exe
innoextract glrg22ww.exe

mv app/ME9.1_5M_Production.bin "${COREBOOT_DIR}/util/me_cleaner"

popd

pushd "${COREBOOT_DIR}/util/me_cleaner"

# Neutralize and shrink Intel ME. Note that this doesn't include
# --soft-disable to set the "ME Disable" or "ME Disable B" (e.g., High
# Assurance Program) bits, as they are defined within the Flash
# Descriptor.
# https://github.com/corna/me_cleaner/wiki/External-flashing#neutralize-and-shrink-intel-me-useful-only-for-coreboot
python me_cleaner.py -r -t -O me_shrinked.bin ME9.1_5M_Production.bin

popd

mv "${COREBOOT_DIR}/util/me_cleaner/me_shrinked.bin" "$1/me.bin"
rm "${COREBOOT_DIR}/util/me_cleaner/"*.bin
fi

if ! echo "${ME_BIN_HASH} $1/me.bin" | sha256sum --check; then
echo "ERROR: SHA256 checksum for me.bin doesn't match."
exit 1
fi
fi
fi
50 changes: 50 additions & 0 deletions blobs/t440p/extract
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
#!/bin/bash

set -e

function usage() {
echo -n \
"Usage: $(basename "$0") path_to_original_rom path_to_output_directory
Extract Intel firmware from the original ROM.
"
}

if [[ "${BASH_SOURCE[0]}" == "$0" ]]; then
if [[ "${1:-}" == "--help" ]]; then
usage
else
if [[ -z "${COREBOOT_DIR}" ]]; then
echo "ERROR: No COREBOOT_DIR variable defined."
exit 1
fi

if [[ -n "$1" ]]; then
pushd "${COREBOOT_DIR}"

cd util/me_cleaner

# Neutralize and shrink Intel ME.
# https://github.com/corna/me_cleaner/wiki/External-flashing#neutralize-and-shrink-intel-me-useful-only-for-coreboot
python me_cleaner.py -S -r -t -d -O out.bin -D ifd_shrinked.bin -M me_shrinked.bin "$1"

mv ifd_shrinked.bin "$2/ifd.bin"
mv me_shrinked.bin "$2/me.bin"
rm ./*.bin

cd ../ifdtool
make

# Extract the Intel Gigabit Ethernet (GbE) firmware from the
# original ROM.
./ifdtool -x "$1"

mv flashregion_3_gbe.bin "$2/gbe.bin"
rm ./*.bin

popd
else
echo "ERROR: You must supply a path to the original ROM."
exit 1
fi
fi
fi
Binary file added blobs/t440p/gbe.bin
Binary file not shown.
Binary file added blobs/t440p/ifd.bin
Binary file not shown.
6 changes: 6 additions & 0 deletions boards/t440p-hotp-maximized/t440p-hotp-maximized.config
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# Inherit the rest from the base T440p config.
include $(pwd)/boards/t440p-maximized/t440p-maximized.config

CONFIG_HOTPKEY=y

export CONFIG_BOARD_NAME="ThinkPad T440p-hotp-maximized"
Comment on lines +1 to +6
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I took a novel approach here. I didn't want to duplicate any config. I realized since this is just a Makefile, we can use the include directive.

60 changes: 60 additions & 0 deletions boards/t440p-maximized/t440p-maximized.config
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
# Configuration for a ThinkPad T440p.
CONFIG_COREBOOT_CONFIG=config/coreboot-t440p.config
# TODO: Make a ThinkPad-common Linux config file.
CONFIG_LINUX_CONFIG=config/linux-t440p.config

export CONFIG_COREBOOT=y
export CONFIG_COREBOOT_VERSION=4.17
export CONFIG_LINUX_VERSION=5.10.5

CONFIG_CRYPTSETUP2=y
CONFIG_FLASHROM=y
CONFIG_FLASHTOOLS=y
CONFIG_GPG2=y
CONFIG_KEXEC=y
CONFIG_UTIL_LINUX=y
CONFIG_LVM2=y
CONFIG_MBEDTLS=y
CONFIG_PCIUTILS=y
CONFIG_POPT=y
CONFIG_QRENCODE=y
CONFIG_TPMTOTP=y
Comment on lines +10 to +21
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These magic values were copied from the X230 config.


# Dependencies for a graphical menu. Enable CONFIG_SLANG and CONFIG_NEWT instead
# for a console-based menu.
CONFIG_CAIRO=y
CONFIG_FBWHIPTAIL=y

CONFIG_LINUX_USB=y

export CONFIG_TPM=y
export CONFIG_BOOTSCRIPT=/bin/gui-init
export CONFIG_BOOT_REQ_HASH=n
export CONFIG_BOOT_REQ_ROLLBACK=n
export CONFIG_BOOT_DEV="/dev/sda1"
export CONFIG_BOARD_NAME="ThinkPad T440p-maximized"
export CONFIG_FLASHROM_OPTIONS="-p internal"

# Make the Coreboot build depend on the following 3rd party blobs:
$(build)/coreboot-$(CONFIG_COREBOOT_VERSION)/$(BOARD)/.build: \
$(pwd)/blobs/haswell/mrc.bin $(pwd)/blobs/t440p/me.bin

$(pwd)/blobs/haswell/mrc.bin:
COREBOOT_DIR="$(build)/$(coreboot_base_dir)" \
$(pwd)/blobs/haswell/obtain-mrc $(pwd)/blobs/haswell

$(pwd)/blobs/t440p/me.bin:
COREBOOT_DIR="$(build)/$(coreboot_base_dir)" \
$(pwd)/blobs/t440p/download-clean-me $(pwd)/blobs/t440p
Comment on lines +38 to +48
Copy link
Contributor Author

Choose a reason for hiding this comment

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

make rule syntax is:

targets : prerequisites
        recipe
        …

Where a "target" is a path to a file that needs to exist. So, we create two targets for mrc.bin and me.bin with recipes that run the corresponding blob scripts. And we make the Coreboot build target (defined elsewhere) depend on these blob targets.

Since a target is a path to a file, if the blobs already exist (e.g., user-supplied blobs), make will skip these recipes.

See: https://www.gnu.org/software/make/manual/make.html#Rule-Syntax


# Haswell boards have an 8 MiB and 4 MiB SPI flash chip. So, we split the
# Coreboot ROM into two files to flash one on each chip.
all: $(board_build)/heads-$(BOARD)-$(HEADS_GIT_VERSION)-bottom.rom
$(board_build)/heads-$(BOARD)-$(HEADS_GIT_VERSION)-bottom.rom: $(board_build)/$(CB_OUTPUT_FILE)
$(call do,DD 8MB,$@,dd of=$@ if=$< bs=65536 count=128 skip=0 status=none)
@sha256sum $@ | tee -a "$(HASHES)"

all: $(board_build)/heads-$(BOARD)-$(HEADS_GIT_VERSION)-top.rom
$(board_build)/heads-$(BOARD)-$(HEADS_GIT_VERSION)-top.rom: $(board_build)/$(CB_OUTPUT_FILE)
$(call do,DD 4MB,$@,dd of=$@ if=$< bs=65536 count=64 skip=128 status=none)
@sha256sum $@ | tee -a "$(HASHES)"
18 changes: 18 additions & 0 deletions config/coreboot-t440p.config
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# CONFIG_USE_BLOBS is not set
CONFIG_VENDOR_LENOVO=y
CONFIG_NO_POST=y
CONFIG_CBFS_SIZE=0x800000
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 value for the CBFS size is arbitrary. Originally, I had totaled the size of all binary blobs, subtracted that from the T440p's ROM size (12 MiB), and used the remaining space as the CBFS size (~11.68 MiB). However, this caused very long RAM initialization times (courtesy of cbmem -t). And, an anecdote in https://groups.google.com/a/chromium.org/g/chromium-os-reviews/c/lUqRrGUoEBY/m/ka7L1f2BS8gJ suggested that this value needs to be a power of 2.

So, I picked a size I expected our Linux payload to fit into that was a power of 2 that I also expected would leave enough space in the ROM for the IFD, ME, GbE, and Coreboot.

Now, it takes less than a second for RAM initialization after flashing/first boot (anecdotally, it seems the MRC needs to be "trained?").

Copy link
Collaborator

@tlaurion tlaurion Jan 17, 2023

Choose a reason for hiding this comment

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

I have the impression that your initial CBFS_SIZE value might have been good, but not matching ifd defined region).

Ifd needs to be modified to take output of neutered ME size while ME region needs to be reduced under ifd, and the freed space added to BIOS region there. That BIOS size would become the size of cbfs region under coreboot config from memory, but will have to find traces again which I was unsuccessful finding last time.

Will try to find that information back since it needs to be part of the porting guide as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have the impression that your initial CBFS_SIZE value might have been good, but not matching ifd defined region).

Ah, okay, this makes sense. I ran me_cleaner on my original ROM again and then used ifdtool to dump the IFD regions:

$ python me_cleaner.py -S -r -t -d -O out.bin -D ifd_shrinked.bin -M me_shrinked.bin ~/projects/t440p/t440p_original.bin 
. . .
$ ./ifdtool -f layout.txt ifd_shrinked.bin && cat layout.txt
00000000:00000fff fd
00021000:00bfffff bios
00003000:00020fff me
00001000:00002fff gbe

Putting this into a notepad (using Soulver), I determined the size of each region and that there is 1 byte of padding between each region:

cbfs_size_math

So maybe 0xBDEFFF is the maximum CBFS size? I will recompile, flash, and see. It would be cool if we could automate this math.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update: 0xBDEFFF did not work. RAM initialization times are back up. ☹️.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

@rbreslow I finally joined coreboot channel and posted my question directly to them here: https://matrix.to/#/!BZxDFuoBnMKnzVZwEp:libera.chat/$N_Nh4qJeU9gquoE9tzjeERYzsxgYB6j79uB6uvnoPdM?via=libera.chat&via=matrix.org&via=foss.wtf

icon on coreboot channel answered. Excerpt:

icon:
it's not end - start but end - start + 1
but that's not why it's slowing down. I assume with the bigger size, not everything in CBFS can be cached
there's CAR (cache as RAM) at 0xff7c0000..0xff7effff. this is used to store data before the DRAM is up
the BIOS region is mapped directly below 4G (0x100000000). so making CBFS too big, it could even conflict with CAR
I don't think this can be changed without also changing the MRC blob
so the best option is really to set CBFS_SIZE to 8MiB
one could still make the rest of the BIOS region available with a custom FMAP. only not as part of CBFS

I replied:

Interesting. Has no impact on ivy/sandy which extends cbfs region to reuse all liberated space from ME (11.5mb for heads maximized boards). But yet again, native ram init there.
Misalignments increased boot time in the past, but those were linked to tpm measurements problems (increasing boot time by 50 seconds, but also on older versions of coreboot).

Then icon replied:

yes, CAR is in 0xfefe0000..0xfefeffff for SNB
that's DCACHE_RAM_BASE/_SIZE in Kconfig btw.
oh, actually it's bigger

nic3-14159 added:

Side note, IIRC the coreboot build system rounds CBFS_SIZE down to 64KiB boundaries


@rbreslow Can you adjust the calculations and test with new CBFS? Might be a limitation of the MRC blob on which we might need to accept caching time (and document properly under board config as current limitation)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tlaurion Here are my cbmem logs from the CBFS_SIZE set 0xBDEFFF: https://gist.github.com/rbreslow/50b45e538437dd10f9e2bf51abd260de. What size would you like me to try now?

Copy link
Collaborator

Choose a reason for hiding this comment

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

So maybe 0xBDEFFF is the maximum CBFS size? I will recompile, flash, and see. It would be cool if we could automate this math.

Basically, the right calculation is taking output of ifdtool -f layout.txt path/to/ifd_output_from_me_cleaner_on_fullrom_backup.rom and parsing the BIOS region, substracting end address to begin address.

@rbreslow Seems like xx30 and xx20 might have been off all along. Redoing.

CONFIG_IFD_BIN_PATH="@BLOB_DIR@/t440p/ifd.bin"
CONFIG_ME_BIN_PATH="@BLOB_DIR@/t440p/me.bin"
CONFIG_GBE_BIN_PATH="@BLOB_DIR@/t440p/gbe.bin"
CONFIG_HAVE_IFD_BIN=y
CONFIG_BOARD_LENOVO_THINKPAD_T440P=y
CONFIG_LINUX_COMMAND_LINE="intel_iommu=igfx_off"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without this, neither Qubes OS nor the Qubes OS installer would start. Presumably, because we're "kexecing" from an already running kernel, we need this set at the Coreboot level? Testing revealed that including intel_iommu=igfx_off in the CONFIG_BOOT_KERNEL_ADD board config option did nothing. And, the Qubes OS default boot option already contains intel_iommu=igfx_off.

See:

Copy link
Collaborator

Choose a reason for hiding this comment

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

Exactly. Otherwise the iommu is configured incorrectly since first kernel boot would try to isolate Intel iommu and the kexec'ed kernel would not be able set differently.

Coreboot defined kernel option defines Heads behavior.

CONFIG_TPM_MEASURED_BOOT=y
CONFIG_HAVE_MRC=y
CONFIG_MRC_FILE="@BLOB_DIR@/haswell/mrc.bin"
CONFIG_HAVE_ME_BIN=y
CONFIG_HAVE_GBE_BIN=y
CONFIG_PAYLOAD_LINUX=y
CONFIG_PAYLOAD_FILE="@BOARD_BUILD_DIR@/bzImage"
CONFIG_LINUX_INITRD="@BOARD_BUILD_DIR@/initrd.cpio.xz"