-
-
Notifications
You must be signed in to change notification settings - Fork 191
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
qemu-coreboot-tpm boards: usage optimizations #1273
qemu-coreboot-tpm boards: usage optimizations #1273
Conversation
boards/qemu-coreboot-fbwhiptail-tpm1-hotp/qemu-coreboot-fbwhiptail-tpm1-hotp.config
Outdated
Show resolved
Hide resolved
boards/qemu-coreboot-fbwhiptail-tpm1-hotp/qemu-coreboot-fbwhiptail-tpm1-hotp.md
Outdated
Show resolved
Hide resolved
QEMU_USB_TOKEN_DEV := -device "usb-host,$(USB_TOKEN)" | ||
endif | ||
|
||
run: $(TPMDIR)/.manufacture $(ROOT_DISK_IMG) $(MEMORY_SIZE_FILE) $(USB_FD_IMG) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only actual difference here is no HOTP right?
Maybe all of the others should include config/coreboot-qemu-tpm1.config
and then override a few variables? There is a lot of business logic relating to provisioning disks, starting TPM, attaching USB token, starting qemu, etc., that I don't want to drift apart.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
user@heads-tests:~/heads$ grep -Rn COREBOOT_CONFIG boards/ | grep tpm1
boards/qemu-coreboot-whiptail-tpm1/qemu-coreboot-whiptail-tpm1.config:9:CONFIG_COREBOOT_CONFIG=config/coreboot-qemu-tpm1.config
boards/qemu-coreboot-fbwhiptail-tpm1-hotp/qemu-coreboot-fbwhiptail-tpm1-hotp.config:11:CONFIG_COREBOOT_CONFIG=config/coreboot-qemu-tpm1.config
boards/qemu-coreboot-fbwhiptail-tpm1/qemu-coreboot-fbwhiptail-tpm1.config:9:CONFIG_COREBOOT_CONFIG=config/coreboot-qemu-tpm1.config
boards/qemu-coreboot-whiptail-tpm1-hotp/qemu-coreboot-whiptail-tpm1-hotp.config:11:CONFIG_COREBOOT_CONFIG=config/coreboot-qemu-tpm1.config
This is the case as of today on master, where Makefile is now dealing with bits permitting generalization:
config/coreboot-qemu-tpm1.config:
CONFIG_PAYLOAD_FILE="@BOARD_BUILD_DIR@/bzImage"
CONFIG_LINUX_INITRD="@BOARD_BUILD_DIR@/initrd.cpio.xz"
So the business logic is still respected under master?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussed separately - deduplicating config would be great by extracting common qemu parts, but we'd like to investigate it in the general context of extracting common config rather than slapdash having one config include another.
- ROOT_DISK_IMG is now dynamic (ROOT_DISK_IMG=/path/to/existing/provisioned/disk.img can be reused across run statements) - Addition of missing boards to cover all use cases - All TPM1 boards rely on common config/coreboot-qemu-tpm1.config - boards/qemu-coreboot-fbwhiptail-tpm1-hotp/qemu-coreboot-fbwhiptail-tpm1-hotp.md has been generalized - all other boards are softlinked to the above for usage
d5a89e9
to
080d439
Compare
Looks good to me, let's merge it 💯 |
@JonathonHall-Purism : funny enough, the reason my tests under #1262 failed for fbwhiptail qemu boards was because of whiptail script modified there under fbwhiptail use case, which depended on readlink which was unavailable under busybox.....
Please review! (Combine with #1272 to see .auditing-0 disappear under /boot)