cvm: consistency in cpuid features and xsave support #2691
Conversation
| 0, | ||
| cpuid::ExtendedFeatureSubleaf1Edx::new() | ||
| .with_avx_vnni_int8(true) | ||
| .with_avx_vnni_int16(true) |
There was a problem hiding this comment.
@chris-oo avx_vnni_int16 is the new bit being filtered in. Lmk if you prefer we don't make this change.
There was a problem hiding this comment.
(I still haven't tested yet that TDX boots)
There was a problem hiding this comment.
Pull request overview
This PR enhances CPUID feature consistency for confidential VMs (CVMs) by ensuring that features requiring XSAVE support are not advertised when the corresponding XSAVE state components are unavailable. The changes add the avx_vnni_int16 field to the CPUID structure and implement XSAVE dependency validation across both common code and TDX-specific paths.
Changes:
- Added
avx_vnni_int16field to ExtendedFeatureSubleaf1Edx structure - Implemented common XSAVE dependency checking that clears AVX/AVX2/AVX512 features when required XSAVE states are missing
- Added TDX-specific XSAVE dependency handling for leaf 7.1 features
- Moved leaf 7.1 feature masking from common masking.rs to TDX-specific code, allowing avx_vnni_int16 to be exposed for TDX
- Added test coverage for XSAVE dependency validation
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| vm/x86/x86defs/src/cpuid.rs | Adds avx_vnni_int16 field to ExtendedFeatureSubleaf1Edx bitfield structure |
| openhcl/virt_mshv_vtl/src/cvm_cpuid/tests/xfem.rs | Adds test verifying CPUID features are cleared when XSAVE support is missing |
| openhcl/virt_mshv_vtl/src/cvm_cpuid/tdx.rs | Implements TDX-specific XSAVE dependency handling and adds avx_vnni_int16 to additional_leaf_mask |
| openhcl/virt_mshv_vtl/src/cvm_cpuid/snp.rs | Adds no-op SNP-specific update_xsave_dependencies implementation |
| openhcl/virt_mshv_vtl/src/cvm_cpuid/mod.rs | Implements common XSAVE dependency checking for AVX/AVX2/AVX512 features and adds trait method for architecture-specific handling |
| openhcl/virt_mshv_vtl/src/cvm_cpuid/masking.rs | Removes leaf 7.1 masking from common code (moved to TDX-specific implementation) |
| let result = cpuid_result(&cpuid, CpuidFunction::VersionAndFeatures, 0); | ||
| assert_eq!(result.ecx & u32::from(version_and_features_clear), 0); | ||
|
|
||
| let result = cpuid_result(&cpuid, CpuidFunction::ExtendedFeatures, 0); | ||
| assert_eq!(result.ebx & extended_features_0_clear.ebx, 0); | ||
| assert_eq!(result.ecx & extended_features_0_clear.ecx, 0); | ||
| assert_eq!(result.edx & extended_features_0_clear.edx, 0); | ||
| } |
There was a problem hiding this comment.
The test should verify that features in ExtendedFeatureSubleaf1 (leaf 7.1) are also being cleared when XSAVE support is missing. Currently, the test only checks leaf 1 (VersionAndFeatures) and leaf 7.0 (ExtendedFeatures subleaf 0), but doesn't verify that avx_vnni, avx_ifma, avx_vnni_int8, avx_vnni_int16, avx_ne_convert, or avx512_bfloat16 from leaf 7.1 are properly cleared. This is important because the PR specifically adds support for clearing these features in TDX environments.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
sluck-msft
force-pushed
the
user/sluck/cpuid-consistency
branch
from
ef0f4bb to
1dcd2c5
Compare
4 participants
Since the host can provide CPUID bits (to a varying degree), check if there are any XSAVE-dependent features where the XSAVE support is not expressed as available (this appears to be validated to varying degrees by the TCBs in question), and update CPUID so that those features are not advertised as available.
On TDX, avx_vnni_int16 was also being filtered out. There doesn't seem to be a reason to do this, so filter it back in.
Tested:
SNP VM boots