Skip to content

Add TD Info as Initialization Header#102

Open
yuehtingchen wants to merge 10 commits intomicrosoft:mainfrom
yuehtingchen:main
Open

Add TD Info as Initialization Header#102
yuehtingchen wants to merge 10 commits intomicrosoft:mainfrom
yuehtingchen:main

Conversation

@yuehtingchen
Copy link

@yuehtingchen yuehtingchen commented Jan 27, 2026

TDX VMs require additional launch time configurations for better flexibility

  • Adds td_info as an optional header in the IGVM file to store TDX specific configurations
  • Add xfam as a parameter in td_info to specify extended processor feature enablement
  • Validate td_info compatibility mask specifies TDX platform

@yuehtingchen yuehtingchen marked this pull request as ready for review January 29, 2026 01:01
@yuehtingchen yuehtingchen requested a review from a team as a code owner January 29, 2026 01:01
pub used_size: u64,
}

/// Optional launch time configurations for VMs running on TDX platform.
Copy link
Member

Choose a reason for hiding this comment

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

Is this really optional if it impacts the measurement of the guest? It's optional in the sense of "if you don't specify this, the VMM may choose whatever value it likes" correct?

Copy link
Author

Choose a reason for hiding this comment

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

Yes. It's optional in the sense that the IGVM file should load even if you don't specify these fields. I don't think all guests care if the measurements are different across different hosts, so some guests may choose not to specify these fields.

Copy link
Member

Choose a reason for hiding this comment

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

But whatever the host has chosen to put for this value, will be reflected in the measurement report right?

Copy link
Author

Choose a reason for hiding this comment

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

Yes that's correct

@luigix25
Copy link
Contributor

luigix25 commented Feb 4, 2026

Hi @yuehtingchen, I have a few comments, just from the git POV, I'm not an IGVM expert so I'll leave that to the maintainers.

  • I see that in this PR you add and revert the same commit, I suggest you to rebase so they cancel each other out.
  • In commit fix compile errs you introduce a problem in dump_igvm.c that you fix in fix c build errs. Why not squashing them together?
  • Same goes for format fixes: why introduce a "problem" and then fix it later? You can do it in one shot.
  • Try to write commit messages that explain why you are doing something. https://www.kernel.org/doc/html/v4.10/process/submitting-patches.html#describe-your-changes I hope this guide helps you.

Thanks!

@chris-oo
Copy link
Member

chris-oo commented Feb 4, 2026

Note that we do squash PRs for this repo, but the feedback is still useful as it helps reviewers logically reason about changes.

@yuehtingchen yuehtingchen reopened this Feb 4, 2026
/// Reserved, must be zero.
pub reserved: u32,
/// XFAM for CPU extended features setting.
pub xfam: u64,
Copy link
Contributor

Choose a reason for hiding this comment

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

should we have fields for mrconfigid, mrowner, mrownerconfig as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, those are attestation fields that will be handled someday by CoRIM. This structure defines only fields that control host construction of the TD and should not define fields that the host does not configure as part of the load process.

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.

5 participants