-
Notifications
You must be signed in to change notification settings - Fork 163
Update stale SNP TODO comments #2699
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
This PR modifies files containing For more on why we check whole files, instead of just diffs, check out the Rustonomicon |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Removes outdated SNP-related TODOs and updates SNP RMP-adjust documentation around VMSA handling.
Changes:
- Removes a stale TODO comment in SNP CPUID extended max function logic.
- Updates
rmpadjust_pagesrustdoc with notes about how VMSA pages are converted.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| openhcl/virt_mshv_vtl/src/cvm_cpuid/snp.rs | Removes an outdated SNP TODO comment in CPUID max-leaf selection. |
| openhcl/hcl/src/ioctl/snp.rs | Updates rmpadjust_pages documentation and modifies behavior around VMSA-related RMPADJUST handling. |
| pub fn rmpadjust_pages( | ||
| &self, | ||
| range: MemoryRange, | ||
| value: SevRmpAdjust, | ||
| terminate_on_failure: bool, | ||
| ) -> Result<(), SnpPageError> { | ||
| if value.vmsa() { | ||
| // TODO SNP: VMSA conversion does not work. | ||
| return Ok(()); | ||
| } | ||
|
|
||
| // SAFETY: TODO SNP FUTURE: For defense in depth it could be useful to prevent |
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR description says this only removes stale TODO comments, but this hunk also removes the if value.vmsa() { return Ok(()); } early-return in rmpadjust_pages, which is a functional behavior change (VMSA adjustments will now be sent to the ioctl). Either restore the previous behavior or update the PR description (and rationale) to reflect the intentional behavior change.
| /// Execute the rmpadjust instruction on the specified memory range. | ||
| /// | ||
| /// The range must not be mapped in the kernel as RAM. | ||
| /// |
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The blank rustdoc line uses /// (with a trailing space). Please remove the trailing whitespace (use /// for a blank doc line) to avoid formatting/lint noise and keep diffs clean.
| /// | |
| /// |
Remove a couple SNP TODO comments that are out of date.