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

Move usermode halt logic into backing-specific logic #114

Merged
merged 3 commits into from
Oct 24, 2024

Conversation

smalis-msft
Copy link
Contributor

@smalis-msft smalis-msft commented Oct 17, 2024

Supersedes #111. Since WHP is the only case in which we can hit this path, make it a bit clearer with a new per-backing method. Open for bikeshedding of course

@smalis-msft smalis-msft requested a review from a team as a code owner October 17, 2024 18:26
@smalis-msft smalis-msft force-pushed the poll_apic-no-bool branch 2 times, most recently from a217241 to 49925de Compare October 21, 2024 20:53
Comment on lines +684 to +686
if T::halt_in_usermode(self, GuestVtl::Vtl0) {
break Poll::Pending;
} else {
Copy link
Contributor

@sluck-msft sluck-msft Oct 23, 2024

Choose a reason for hiding this comment

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

So for x64 the halt gets handled here, but in e.g. snp it gets handled in run_vp_snp? Do you know what the reasoning is for the difference in location? Wondering if we should try to be consistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For x64 with apic emulation it gets handled here, which is only the case in test scenarios (WHP). For all other scenarios we call set_halted on the runner and let the kernel manage it for us, as that's more efficient.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not entirely familiar with why we can't do that for WHP, I just tried to preserve existing behavior here.

Copy link
Collaborator

@mebersol mebersol left a comment

Choose a reason for hiding this comment

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

:shipit:

@smalis-msft smalis-msft merged commit 10a0118 into microsoft:main Oct 24, 2024
19 checks passed
@smalis-msft smalis-msft deleted the poll_apic-no-bool branch October 24, 2024 14:32
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.

3 participants