Compile time flag to disable secure AVIC#3505
Open
jennagoddard wants to merge 5 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a debug-oriented switch to disable Secure AVIC for SNP guests to enable local testing on Genoa/Milan, while propagating the option through Flowey and introducing a Cargo feature to alter SNP behavior at compile time.
Changes:
- Add
--disable-secure-avictoigvmfilegento patch SNP guest configs in the manifest tosecure_avic = disabled. - Add and plumb a
disable_secure_avicflag through Flowey build pipelines, with a guard against use in release builds for local jobs. - Introduce a
disable_secure_avicCargo feature that forces Secure AVIC off in SNP code paths.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| vm/loader/igvmfilegen/src/main.rs | Adds CLI flag to rewrite manifest SNP configs to disable Secure AVIC |
| openhcl/virt_mshv_vtl/src/processor/snp/mod.rs | Adds feature-gated logic to force Secure AVIC off and adjusts assertions/feature setup |
| openhcl/virt_mshv_vtl/Cargo.toml | Introduces disable_secure_avic feature |
| openhcl/underhill_entry/Cargo.toml | Re-exports feature through crate feature chain |
| openhcl/underhill_core/Cargo.toml | Re-exports feature through crate feature chain |
| openhcl/openvmm_hcl/Cargo.toml | Re-exports feature through crate feature chain |
| flowey/flowey_lib_hvlite/src/run_igvmfilegen.rs | Plumbs optional CLI flag into igvmfilegen invocation |
| flowey/flowey_lib_hvlite/src/build_openhcl_igvm_from_recipe.rs | Adds flag to request and forwards it to igvmfilegen node |
| flowey/flowey_lib_hvlite/src/_jobs/local_build_igvm.rs | Adds CLI/customization support and blocks flag for release builds |
| flowey/flowey_lib_hvlite/src/_jobs/local_build_and_run_nextest_vmm_tests.rs | Sets default disable_secure_avic: false |
| flowey/flowey_lib_hvlite/src/_jobs/build_and_publish_openhcl_igvm_from_recipe.rs | Sets default disable_secure_avic: false |
| flowey/flowey_hvlite/src/pipelines/build_igvm.rs | Exposes --disable-secure-avic on build pipeline CLI |
Comment on lines
+851
to
857
| #[cfg(not(feature = "disable_secure_avic"))] | ||
| if vtl == GuestVtl::Vtl0 && sev_status.secure_avic() { | ||
| vmsa.sev_features_mut().set_secure_avic(true); | ||
| vmsa.sev_features_mut().set_guest_intercept_control(true); | ||
| } else { | ||
| vmsa.sev_features_mut().set_alternate_injection(true); | ||
| } |
| /// Additional debug validation when building IGVM files | ||
| #[clap(long)] | ||
| debug_validation: bool, | ||
| /// Override secure AVIC to disabled for debug SNP guest configs |
Comment on lines
+459
to
462
| #[cfg(feature = "disable_secure_avic")] | ||
| let secure_avic = false; | ||
| #[cfg(not(feature = "disable_secure_avic"))] | ||
| let secure_avic = sev_status.secure_avic(); |
Comment on lines
+70
to
+74
| let disable_secure_avic_flag = if disable_secure_avic { | ||
| "--disable-secure-avic" | ||
| } else { | ||
| "" | ||
| }; |
smalis-msft
reviewed
May 18, 2026
smalis-msft
reviewed
May 18, 2026
| mi-secure = ["underhill_entry/mi-secure"] | ||
|
|
||
| # Disable secure AVIC support for SNP. | ||
| disable_secure_avic = ["underhill_entry/disable_secure_avic"] |
Contributor
There was a problem hiding this comment.
If we do keep this as a feature, you'll probably need to add it to the closed source openvmm_hcl_msft too
Comment on lines
+852
to
+858
| { | ||
| if vtl == GuestVtl::Vtl0 && sev_status.secure_avic() { | ||
| vmsa.sev_features_mut().set_secure_avic(true); | ||
| vmsa.sev_features_mut().set_guest_intercept_control(true); | ||
| } else { | ||
| vmsa.sev_features_mut().set_alternate_injection(true); | ||
| } |
Comment on lines
+459
to
+463
| #[cfg(feature = "disable_secure_avic")] | ||
| let secure_avic = false; | ||
| #[cfg(not(feature = "disable_secure_avic"))] | ||
| let secure_avic = sev_status.secure_avic(); | ||
| tracing::info!(CVM_ALLOWED, ?secure_avic, "Secure AVIC status"); |
| } | ||
|
|
||
| if disable_secure_avic && (release_cfg || release) { | ||
| anyhow::bail!("--disable-secure-avic cannot be used with release builds."); |
Comment on lines
+131
to
+135
| /// Disable secure AVIC support for SNP. This adds the | ||
| /// `disable_secure_avic` cargo feature and sets `secure_avic` to | ||
| /// `disabled` in the IGVM manifest. | ||
| #[clap(long)] | ||
| pub disable_secure_avic: bool, |
| /// Additional debug validation when building IGVM files | ||
| #[clap(long)] | ||
| debug_validation: bool, | ||
| /// Override secure AVIC to disabled for debug SNP guest configs |
smalis-msft
reviewed
May 20, 2026
| vmsa.sev_features_mut().set_guest_intercept_control(true); | ||
| } else { | ||
| vmsa.sev_features_mut().set_alternate_injection(true); | ||
| #[cfg(not(feature = "disable_secure_avic"))] |
Contributor
There was a problem hiding this comment.
Don't we still need to set alternate injection when we're not doing secure_avic?
Contributor
Author
There was a problem hiding this comment.
well its not secure but we aren't shipping with alternate injection
Contributor
There was a problem hiding this comment.
Do we want to start deleting alternate injection code then?
Contributor
Author
There was a problem hiding this comment.
I discussed that with Mike a few months ago and we decided no but I don't recall why
smalis-msft
reviewed
May 20, 2026
| recipe: recipe_to_use, | ||
| custom_target: None, | ||
| extra_features: BTreeSet::new(), | ||
| disable_secure_avic: false, |
Contributor
There was a problem hiding this comment.
Do we want to plumb the flag through for the vmm test command too? That way people can run tests on non-secure-avic machines
Contributor
Author
There was a problem hiding this comment.
that is a good question I'll look into it
Contributor
Author
There was a problem hiding this comment.
I'll do this as a separate change
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This enables local testing on debug builds on Genoa/Milan hardware using a build flag:
cargo xflowey build-igvm x64-cvm --disable-secure-avic
A build time cfg feature is used to compile out this option on release builds.