Skip to content
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

virt_mshv_vtl: Share common processing between poll_apic implementations #630

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

smalis-msft
Copy link
Contributor

@smalis-msft smalis-msft commented Jan 8, 2025

This is far from the cleanest code I've ever written, but it accomplishes my goal of sharing the tricky and involved checks between all of our poll_apic implementations. Having something like this would've saved me some pain with TDX recently. Definitely open for feedback on design and bikeshedding, I just wanted to get out a min viable refactoring, that created as small a diff as I could, to see if it was doable.

Still needs CVM testing

@smalis-msft smalis-msft requested a review from a team as a code owner January 8, 2025 18:17

pub(crate) fn poll_apic_core<T: ApicBacking>(
processor: &mut T,
scan: impl Fn(&mut T) -> ApicWork,
Copy link
Contributor Author

@smalis-msft smalis-msft Jan 8, 2025

Choose a reason for hiding this comment

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

These closures could all move onto ApicBacking if we want, just creates a larger diff.

@@ -392,61 +392,22 @@ impl BackingPrivate for SnpBacked {
vtl: GuestVtl,
scan_irr: bool,
) -> Result<(), UhRunVpError> {
// Check for interrupt requests from the host.
// TODO SNP GUEST VSM supporting VTL 1 proxy irrs requires kernel changes
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This comment (and the corresponding one in TDX) are gone now, as John and I decided that we don't plan on implementing this in the near future. Just doesn't seem to be worthwhile.

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.

1 participant