Skip to content

Conversation

maheeraeron
Copy link
Contributor

UEFI is already now responsible for locking the TPM platform hierarchy, which will occur before the End-Of-Dxe event (i.e., the PH will be locked before we reach boot devices).

Thus, we can remove this code in OpenHCL that does this.

@maheeraeron maheeraeron requested a review from a team as a code owner October 9, 2025 23:00
@Copilot Copilot AI review requested due to automatic review settings October 9, 2025 23:00
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR removes TPM platform hierarchy control code from OpenHCL, as UEFI is now responsible for locking the TPM platform hierarchy before the End-Of-Dxe event.

  • Removes the hierarchy_control method and related TPM command structures
  • Eliminates the platform hierarchy clearing logic during TPM initialization
  • Removes associated test coverage for hierarchy control functionality

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
vm/devices/tpm/src/tpm_helper.rs Removes hierarchy_control helper method and its corresponding test
vm/devices/tpm/src/tpm20proto.rs Removes HierarchyControlCmd and HierarchyControlReply protocol structures
vm/devices/tpm/src/lib.rs Removes platform hierarchy clearing call from TPM initialization

@maheeraeron maheeraeron requested a review from stunes-ms October 9, 2025 23:02
@maheeraeron
Copy link
Contributor Author

maheeraeron commented Oct 9, 2025

NOTE: We should onboard a TPM test for OpenHCL Linux guests to validate that the TPM platform hierarchy is still locked after boot.

The simplest way to do this is to have petri run this command on the guest:

tpm2_createprimary -C p -G rsa -c primary.ctx

If successful, we should expect an error saying `Error: unable to run tpm2_createprimary) with an error code proving that it was due to the PH being disabled.

@smalis-msft @tjones60, would the above proposal require us to install something in our Linux guests?

stunes-ms
stunes-ms previously approved these changes Oct 9, 2025
@stunes-ms
Copy link
Contributor

Looks good to me! I'll leave it to the OpenVMM experts to decide if we should remove the code in tpm20proto and tpm_helper, or just make Clippy not complain about it being unused.

Copy link

github-actions bot commented Oct 9, 2025

Copy link

@maheeraeron maheeraeron changed the title Don't disable TPM platform hierarchy tpm: Don't disable TPM platform hierarchy Oct 10, 2025
Copy link

smalis-msft
smalis-msft previously approved these changes Oct 10, 2025
Copy link
Contributor

@smalis-msft smalis-msft left a comment

Choose a reason for hiding this comment

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

LGTM, though I can't speak to the TPM python code itself.

stunes-ms
stunes-ms previously approved these changes Oct 10, 2025
Copy link
Contributor

@stunes-ms stunes-ms left a comment

Choose a reason for hiding this comment

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

Python test looks good, AFAICT.

@maheeraeron maheeraeron dismissed stale reviews from stunes-ms and smalis-msft via 4c12e81 October 13, 2025 17:26
@maheeraeron maheeraeron enabled auto-merge (squash) October 13, 2025 17:28
@maheeraeron maheeraeron merged commit 22c3642 into microsoft:main Oct 13, 2025
28 checks passed
@maheeraeron maheeraeron deleted the user/maheeraeron/dont-disable-tpm-ph branch October 13, 2025 19:02
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.

3 participants