-
Notifications
You must be signed in to change notification settings - Fork 164
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
Enable TPM in uefi #2624
Enable TPM in uefi #2624
Conversation
013f763
to
fd0982a
Compare
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 qemu parts should be ok, but I don't know the impact of setting the options in pkg/uefi/build.sh - can this have an impact when running on bare metal? @rvs can you take a look?
@rvs can you please review this? Without this patch PCRs are not filling when we run EVE-OS in qemu using eve-uefi package. |
I've been using the same patch for UEFI for a while and I can confirm it is working |
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.
I believe we should be fine when it comes to rebuilding Tianocore with these flags. However, I'm not quite a fan of how it got hooked up to the Makefile logic. See comments.
Makefile
Outdated
(echo 'set devicetree="(hd0,msdos1)/eve.dtb"' ; echo 'set rootfs_root=/dev/vdb' ; echo 'set root=hd1' ; echo 'export rootfs_root' ; echo 'export devicetree' ; echo 'configfile /EFI/BOOT/grub.cfg' ) > $(EFI_PART)/BOOT/grub.cfg | ||
$(QEMU_SYSTEM) $(QEMU_OPTS) -drive file=$(ROOTFS_IMG),format=raw -drive file=fat:rw:$(EFI_PART)/..,label=CONFIG,id=uefi-disk,format=vvfat | ||
$(QUIET): $@: Succeeded | ||
|
||
run-grub: $(BIOS_IMG) $(UBOOT_IMG) $(EFI_PART) $(DEVICETREE_DTB) | ||
run-grub: $(BIOS_IMG) $(UBOOT_IMG) $(EFI_PART) $(DEVICETREE_DTB) swtpm |
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.
Wait, why do we need swtpm in all these targets? I say -- let's make it conditional at least.
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.
Condition is inside swtpm target. I want to run swtpm as part of make TPM=y run...
command.
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.
My point is that it would me much nicer if this followed (some) of the other makefile logic and was a SWTPM variable that would resolve to an empty string conditionally.
I understand that this is highly subjective -- I just absolutely dislike mixing these types of "styles" in Makefile (makefiles are ugly as it is and super convoluted).
To use TPM devices with qemu we should enable support inside uefi Signed-off-by: Petr Fedchenkov <giggsoff@gmail.com>
fd0982a
to
179af52
Compare
Makefile
Outdated
(echo 'set devicetree="(hd0,msdos1)/eve.dtb"' ; echo 'set rootfs_root=/dev/vdb' ; echo 'set root=hd1' ; echo 'export rootfs_root' ; echo 'export devicetree' ; echo 'configfile /EFI/BOOT/grub.cfg' ) > $(EFI_PART)/BOOT/grub.cfg | ||
$(QEMU_SYSTEM) $(QEMU_OPTS) -drive file=$(ROOTFS_IMG),format=raw -drive file=fat:rw:$(EFI_PART)/..,label=CONFIG,id=uefi-disk,format=vvfat | ||
$(QUIET): $@: Succeeded | ||
|
||
run-grub: $(BIOS_IMG) $(UBOOT_IMG) $(EFI_PART) $(DEVICETREE_DTB) | ||
run-grub: $(BIOS_IMG) $(UBOOT_IMG) $(EFI_PART) $(DEVICETREE_DTB) swtpm |
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.
My point is that it would me much nicer if this followed (some) of the other makefile logic and was a SWTPM variable that would resolve to an empty string conditionally.
I understand that this is highly subjective -- I just absolutely dislike mixing these types of "styles" in Makefile (makefiles are ugly as it is and super convoluted).
Makefile
Outdated
# Use tpm (any non-empty value will trigger using it) | ||
TPM= |
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.
Should we document this in the markdown file which has the examples of how to build and run?
Signed-off-by: Petr Fedchenkov <giggsoff@gmail.com>
179af52
to
c550d14
Compare
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.
Run eden again
To use TPM devices with qemu (when we use uefi provided by eve-uefi package) we should enable support inside uefi. Right now I can see zero values inside PCRs when I try to use eve-uefi.
Also this PR enables TPM option in Makefile to use swtpm as TPM device for Qemu.
Signed-off-by: Petr Fedchenkov giggsoff@gmail.com