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.

Sign up for GitHub

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

Jump to bottom

Check that windows version is supported #110

Merged
merged 5 commits into from
Dec 18, 2024
Merged

Check that windows version is supported #110

merged 5 commits into from
Dec 18, 2024

Conversation

simongdavies
Copy link
Contributor

Adds checks to ensure that when running on Windows the version of Windows is supported.

On Windows hyperlight requires the API WHvMapGpaRange2 which only exists on Windows 11 and Windows Server 2022 (or later).

There are multiple commits in this PR each with their own description.

Fixes #76

@simongdavies simongdavies added the kind/bug Something isn't working label Dec 12, 2024
danbugs
danbugs previously approved these changes Dec 13, 2024
View reviewed changes
Copy link
Contributor

@danbugs danbugs left a comment

Choose a reason for hiding this comment

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

LGTM. Might just want to rebase squash commits for related to formatting, clippy, and spelling issues.

dblnz
dblnz previously approved these changes Dec 13, 2024
View reviewed changes
@hyperlight-dev hyperlight-dev deleted a comment from hyperlight-prow bot Dec 16, 2024
@simongdavies
Remove unnecessary check for hypervisor present
0e8fbf1 
Check for hypervisor presence is done in the hypervisor_handler module before trying to create the driver

Signed-off-by: Simon Davies <[email protected]>
@simongdavies simongdavies force-pushed the fix-hypervisor-handler branch from dc63dc6 to a00b180 Compare December 17, 2024 10:58
@hyperlight-dev hyperlight-dev deleted a comment from hyperlight-prow bot Dec 17, 2024
@hyperlight-dev hyperlight-dev deleted a comment from hyperlight-prow bot Dec 17, 2024
@simongdavies simongdavies changed the title Fix hypervisor handler Check that windows version is supported Dec 17, 2024
@simongdavies simongdavies force-pushed the fix-hypervisor-handler branch 3 times, most recently from 1ccdb4d to bcee12c Compare December 17, 2024 12:11
ludfjig
ludfjig previously approved these changes Dec 17, 2024
View reviewed changes
Copy link
Contributor

@ludfjig ludfjig left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM but I am slightly confused. With the new dynamic loading of the function, do we support windows 10 or not? Looks like we return an error in uninitialized.rs if we are not win11, so what's the point of that?

@marosset
Copy link
Contributor

marosset commented Dec 17, 2024 •
edited
Loading

LGTM but I am slightly confused. With the new dynamic loading of the function, do we support windows 10 or not? Looks like we return an error in uninitialized.rs if we are not win11, so what's the point of that?

On Windows hyperlight requires the API WHvMapGpaRange2 which only exists on Windows 11 and Windows Server 2022 (or later). (from the PR description)

marosset
marosset reviewed Dec 17, 2024
View reviewed changes
README.md Outdated Show resolved Hide resolved
marosset
marosset previously approved these changes Dec 17, 2024
View reviewed changes
Copy link
Contributor

@marosset marosset left a comment

Choose a reason for hiding this comment

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

LGTM

@simongdavies
Copy link
Contributor Author

LGTM but I am slightly confused. With the new dynamic loading of the function, do we support windows 10 or not? Looks like we return an error in uninitialized.rs if we are not win11, so what's the point of that?

Previously when hyperlight was loaded on older Windows versions the function WHvMapGpaRange2 was not found resulting in the host encountering an entry point not found error, this error is not trappable since it happens before any hyperlight code is executed, now we resolve the function dynamically it means that we can detect old versions of windows when attempting to create a sandbox and we can return an appropriate error

@simongdavies simongdavies dismissed stale reviews from marosset and ludfjig via 8b6b25c December 17, 2024 19:42
@simongdavies
Check that Windows version is 11 or Server 2022 or later
63ceac0 
Adds a check when creating a new uninitialisedsandbox to ensure that we are running on at least Windows 11 or Windows Server 2022

Signed-off-by: Simon Davies <[email protected]>
@simongdavies
Dynamically resolve WHvMapGpaRange2 function
9839135 
Resolve the WHvMapGpaRange2 function dynamically so that we can load on earlier versions of windows that are not supported

Signed-off-by: Simon Davies <[email protected]>
@simongdavies
Fixes an issue where errors do not get propogated correctly when the …
72ddd4d 
…hypervisor handler thread returns an error.

The hypervisor handler thread can return an error rather than send a message, in this case the error returned would be a timeout rather than the real error, this change ensures that we check for an error in the hypervisor handler thread when we time out and return that if it exists

Signed-off-by: Simon Davies <[email protected]>
@simongdavies
Increase MSRV to 1.80
c3121ae 
This change uses take_if method on Option which is stable since 1.80

Signed-off-by: Simon Davies <[email protected]>
@simongdavies simongdavies force-pushed the fix-hypervisor-handler branch from 8b6b25c to c3121ae Compare December 17, 2024 19:45
@simongdavies simongdavies merged commit c3121ae into hyperlight-dev:main Dec 18, 2024
25 checks passed
@simongdavies simongdavies mentioned this pull request Jan 21, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marosset marosset marosset approved these changes

@dblnz dblnz dblnz approved these changes

@ludfjig ludfjig ludfjig approved these changes

@danbugs danbugs danbugs left review comments

Assignees
No one assigned
Labels
approved kind/bug Something isn't working lgtm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[hyperlight-host] detect presence of WHP for Windows
5 participants
@simongdavies @marosset @ludfjig @dblnz @danbugs