Skip to content

Make interrupt counters visible in release for cvms#3191

Draft
sluck-msft wants to merge 1 commit intomicrosoft:mainfrom
sluck-msft:inspect-int-counters
Draft

Make interrupt counters visible in release for cvms#3191
sluck-msft wants to merge 1 commit intomicrosoft:mainfrom
sluck-msft:inspect-int-counters

Conversation

@sluck-msft
Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI review requested due to automatic review settings April 3, 2026 20:38
Copy link
Copy Markdown
Contributor

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 adjusts the inspect sensitivity metadata across the CVM (confidential VM) path so that interrupt/LAPIC-related counters can be surfaced in release diagnostics, while still respecting the sensitivity filtering model used by Underhill/OpenHCL.

Changes:

  • Mark LAPIC/APIC interrupt counters (and the relevant containment chain) as Safe in inspect so they appear under SensitivityLevel::Safe requests.
  • Add per-state-unit inspect sensitivity control in vmm_core::state_unit, and set the partition unit’s inspect sensitivity to Safe in Underhill.
  • Mark selected top-level inspect fields (e.g., VP list) as Safe so Safe-only inspection can traverse into CVM-safe subtrees.

Reviewed changes

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

Show a summary per file
File Description
vmm_core/virt_support_apic/src/lib.rs Marks Local APIC stats/counters as #[inspect(safe)] so interrupt counters can be visible under Safe inspection.
vmm_core/state_unit/src/lib.rs Adds inspect_sensitivity to state units and uses sensitivity_child to gate each unit’s inspect subtree.
vmm_core/src/partition_unit/vp_set.rs Marks the VP list field as Safe to allow Safe inspection to enumerate VP nodes.
openhcl/virt_mshv_vtl/src/processor/tdx/mod.rs Attempts to mark the CVM subtree safe at the TDX backing level (but note flatten+sensitivity limitation).
openhcl/virt_mshv_vtl/src/processor/snp/mod.rs Attempts to mark the CVM subtree safe at the SNP backing level (but note flatten+sensitivity limitation).
openhcl/virt_mshv_vtl/src/processor/mod.rs Marks the backing field as Safe and marks LAPIC state’s LocalApic as Safe.
openhcl/virt_mshv_vtl/src/lib.rs Marks CVM LAPIC array as Safe to expose LAPIC state that is explicitly marked safe.
openhcl/underhill_core/src/worker.rs Sets the partition state unit’s inspect sensitivity to SensitivityLevel::Safe.
openhcl/underhill_core/src/lib.rs Marks the VM worker handle as Safe for inspection.

flush_page: user_driver::memory::MemoryBlock,

#[inspect(flatten)]
#[inspect(flatten, safe)]
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

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

#[inspect(flatten, safe)] is misleading here: the inspect derive currently ignores sensitivity attributes on flatten fields (flatten just emits resp.merge(...)). This means safe has no effect, and only inner fields explicitly marked safe will be visible under a SensitivityLevel::Safe request. Consider removing safe from this attribute, or (if you need the whole subtree to be safe) wrap it as a named child via Response::sensitivity_child(...)/a non-flatten field so the sensitivity is actually applied.

Suggested change
#[inspect(flatten, safe)]
#[inspect(flatten)]

Copilot uses AI. Check for mistakes.
general_stats: VtlArray<GeneralStats, 2>,
exit_stats: VtlArray<ExitStats, 2>,
#[inspect(flatten)]
#[inspect(flatten, safe)]
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

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

#[inspect(flatten, safe)] does not currently apply safe sensitivity: the inspect derive ignores sensitivity attributes on flatten fields and only emits resp.merge(...). If the intent is to make this subtree visible for SensitivityLevel::Safe requests, you’ll need to mark the specific inner fields as #[inspect(safe)] (as done for LAPIC stats) or restructure to use a sensitivity-aware child/field instead of flatten. Otherwise, consider dropping safe here to avoid confusion.

Suggested change
#[inspect(flatten, safe)]
#[inspect(flatten)]

Copilot uses AI. Check for mistakes.
Comment on lines +854 to +858
/// Sets the sensitivity level for this unit's inspect data. Defaults to Unspecified.
pub fn inspect_sensitivity(mut self, sensitivity: inspect::SensitivityLevel) -> Self {
self.inspect_sensitivity = sensitivity;
self
}
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

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

This change introduces a new public builder API (UnitBuilder::inspect_sensitivity) and changes the inspect tree construction to use sensitivity_child(...), but there are no unit tests exercising the sensitivity behavior (e.g., that SensitivityLevel::Safe requests include Safe units and exclude Unspecified ones). Adding a focused test would help prevent regressions in inspect filtering semantics.

Copilot uses AI. Check for mistakes.
@benhillis benhillis added the enhancement New feature or request label Apr 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants