Skip to content

igvm: Update SevVmsa structure definition. Pad to full 4K.#109

Merged
chris-oo merged 1 commit into
microsoft:mainfrom
RARelph:main
Mar 23, 2026
Merged

igvm: Update SevVmsa structure definition. Pad to full 4K.#109
chris-oo merged 1 commit into
microsoft:mainfrom
RARelph:main

Conversation

@RARelph

@RARelph RARelph commented Mar 3, 2026

Copy link
Copy Markdown
Contributor

Fixes #108 - Updates SevVmsa with documented VMSA fields and extends structure to 4K page size.

@RARelph RARelph requested a review from a team as a code owner March 3, 2026 16:24
@chris-oo chris-oo self-assigned this Mar 3, 2026
@RARelph

RARelph commented Mar 3, 2026

Copy link
Copy Markdown
Contributor Author

@microsoft-github-policy-service agree company="AMD"

@chris-oo

chris-oo commented Mar 4, 2026

Copy link
Copy Markdown
Member

Jon and I discussed this offline that I think it might make more sense to move away from defining some of these architectural definitions in the igvm and igvm_defs crate themselves, and defer to just being an opaque type outside of bits we need within IGVM itself. For example, we think that we might need sev_features for some CoRIM validation in the future, but we'd mark the rest of the fields as reserved, and leave it as convertible to a 4K u8 slice via IntoBytes/FromBytes.

I think this would apply to quite a few things in this crate so I need to sit down and find some time to refactor this, but would mean every time hardware changes/adds a new feature, we're not required to add all these definitions because IGVM shouldn't be the authoritative definition for specific hardware.

This does mean consumers of this crate will need to carry their own definition of hardware specific fields, but I think that's fine. I wonder if we should have a snp_defs crate in this case that consumers can use?

Thoughts?

@RARelph

RARelph commented Mar 4, 2026

Copy link
Copy Markdown
Contributor Author

I think moving architecture specifics to an architecture-specific crate makes sense. Sounds like more work... but it makes sense.

@chris-oo

chris-oo commented Mar 6, 2026

Copy link
Copy Markdown
Member

Mainly, I don't want this crate to become a defacto source for hardware specific definitions. Even if we might use a few bit definitions, they should be stable but we shouldn't have to need to update IGVM's definition for a new hardware feature.

@chris-oo chris-oo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

one small comment then i think it looks fine

Comment thread igvm/src/lib.rs Outdated
Renders the check for non-zero bytes past end of SevVmsa useless.
@chris-oo chris-oo merged commit fc0fa3f into microsoft:main Mar 23, 2026
6 checks passed
chris-oo pushed a commit to microsoft/openvmm that referenced this pull request Jul 10, 2026
Bump the workspace igvm/igvm_defs dependencies to microsoft/igvm rev
3d1a950116d624746bf3342e09ea0a3416d89dd3 (current main tip). This pulls
in:

- [PR #109](microsoft/igvm#109) which pads
`SevVmsa` out to a full 4 KiB page, whereas `x86defs::snp::SevVmsa`
produced by the VP context builder is the architectural 1648-byte
structure.
- [PR #122](microsoft/igvm#122) which adds CoRIM
launch-measurement APIs.
- New `AArch64Register` variants (X2-X7) and a new
`IgvmDirectiveHeader::AArch64CcaVpContext` variant.

Adapt local callers:

- igvmfilegen's `import_pages` SNP branch zero-pads the incoming VMSA up
to the igvm crate's expected size before calling
`SevVmsa::read_from_bytes`, and the strict size check is relaxed to
allow any input no larger than the padded size.
- `vm/loader` and `vmm_core/vm_loader` mirror the new `AArch64Register`
X2-X7 variants.
- `openvmm_core`'s IGVM directive matcher adds `AArch64CcaVpContext`
arms (todo-stubbed; no callers exercise CCA yet).

This is a no-op for current functionality and is a prerequisite for the
upcoming CoRIM endorsement support.

Signed-off-by: Ming-Wei Shih <mishih@microsoft.com>
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.

SevVmsa is incomplete

2 participants