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

Introduce generic hypercall ioctl #128

Merged
merged 6 commits into from
Apr 23, 2024

Conversation

NunoDasNeves
Copy link
Collaborator

@NunoDasNeves NunoDasNeves commented Jan 16, 2024

Summary of the PR

This PR introduces the generic hypercall IOCTL - MSHV_ROOT_HVCALL.
It can be used on both the partition and vp fds.

This IOCTL reduces the number of IOCTLs needed in the kernel interface, reducing maintenance and making the kernel code more upstreamable.

This PR introduces the IOCTL and implements some use-cases for it, but it doesn't replace the default implementations so it won't affect downstream.

Requirements

Before submitting your PR, please make sure you addressed the following
requirements:

  • All commits in this PR are signed (with git commit -s), and the commit
    message has max 60 characters for the summary and max 75 characters for each
    description line.
  • All added/changed functionality has a corresponding unit/integration
    test.
  • All added/changed public-facing functionality has entries in the "Upcoming
    Release" section of CHANGELOG.md (if no such section exists, please create one).
  • Any newly added unsafe code is properly documented.

@NunoDasNeves NunoDasNeves force-pushed the nudasnev/generic-hypercall-ioctl branch from 4ec6798 to 5b8f112 Compare January 17, 2024 17:45
@russell-islam
Copy link
Collaborator

Please update the subject line of the PR with more meaningful one.

@NunoDasNeves NunoDasNeves force-pushed the nudasnev/generic-hypercall-ioctl branch 2 times, most recently from cd9e047 to ee0cfa7 Compare January 17, 2024 23:32
@NunoDasNeves NunoDasNeves changed the title Nudasnev/generic hypercall ioctl Introduce generic hypercall ioctl, replace several ioctls with it Jan 17, 2024
@NunoDasNeves NunoDasNeves force-pushed the nudasnev/generic-hypercall-ioctl branch from ee0cfa7 to af04ea1 Compare January 18, 2024 17:50
mshv-bindings/src/hvcall.rs Outdated Show resolved Hide resolved
mshv-bindings/src/hvcall.rs Show resolved Hide resolved
mshv-bindings/src/hvcall.rs Show resolved Hide resolved
mshv-ioctls/src/ioctls/vcpu.rs Show resolved Hide resolved
@russell-islam
Copy link
Collaborator

Overall looks good functionally. Those macros are a bit hard to understand, If you could provide some example with data it would be good for the reviewers.

@NunoDasNeves NunoDasNeves force-pushed the nudasnev/generic-hypercall-ioctl branch from af04ea1 to fc82497 Compare January 20, 2024 00:19
@NunoDasNeves NunoDasNeves force-pushed the nudasnev/generic-hypercall-ioctl branch 3 times, most recently from 30759b3 to 1719d70 Compare April 3, 2024 22:20
@NunoDasNeves NunoDasNeves force-pushed the nudasnev/generic-hypercall-ioctl branch 5 times, most recently from ee8b081 to dc67278 Compare April 9, 2024 22:39
@NunoDasNeves NunoDasNeves changed the title Introduce generic hypercall ioctl, replace several ioctls with it [WIP] Introduce generic hypercall ioctl, replace several ioctls with it Apr 9, 2024
@NunoDasNeves NunoDasNeves force-pushed the nudasnev/generic-hypercall-ioctl branch from dc67278 to 77d4f14 Compare April 16, 2024 20:52
@NunoDasNeves NunoDasNeves changed the title [WIP] Introduce generic hypercall ioctl, replace several ioctls with it [WIP] Introduce generic hypercall ioctl Apr 17, 2024
@NunoDasNeves NunoDasNeves force-pushed the nudasnev/generic-hypercall-ioctl branch 3 times, most recently from 04731d9 to 2c3696d Compare April 18, 2024 22:29
@NunoDasNeves NunoDasNeves changed the title [WIP] Introduce generic hypercall ioctl Introduce generic hypercall ioctl Apr 18, 2024
Don't use "EIO" as a special code for determining whether there is
a hypercall error. Simply check if there is a hypercall error code.

Signed-off-by: Nuno Das Neves <nudasnev@microsoft.com>
Signed-off-by: Nuno Das Neves <nudasnev@microsoft.com>
Signed-off-by: Nuno Das Neves <nudasnev@microsoft.com>
Signed-off-by: Nuno Das Neves <nudasnev@microsoft.com>
This will be used with the generic hvcall ioctl for vcpus.

Signed-off-by: Nuno Das Neves <nudasnev@microsoft.com>
@NunoDasNeves NunoDasNeves force-pushed the nudasnev/generic-hypercall-ioctl branch from 2c3696d to 9ead785 Compare April 18, 2024 22:36
@NunoDasNeves
Copy link
Collaborator Author

@jinankjain @russell-islam @liuw This PR is reworked and ready for another round of review.
The most notable change is that no downstream code will be affected. This PR just introduces some of the hvcall_ versions of IOCTLs, and basic tests for them.

@NunoDasNeves
Copy link
Collaborator Author

@russell-islam @jinankjain @liuw ping

Copy link
Collaborator

@russell-islam russell-islam left a comment

Choose a reason for hiding this comment

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

LGTM. Minor typos

mshv-ioctls/src/ioctls/vcpu.rs Outdated Show resolved Hide resolved
mshv-ioctls/src/ioctls/vcpu.rs Outdated Show resolved Hide resolved
mshv-ioctls/src/ioctls/vcpu.rs Outdated Show resolved Hide resolved
While leaving existing implementations untouched.

Add:
- VmFd::hvcall_set_partition_property
- VcpuFd::hvcall_translate_gva
- VcpuFd::hvcall_get_cpuid_values
- VcpuFd::hvcall_get_reg
- VcpuFd::hvcall_set_reg

Add tests for the above.

Signed-off-by: Nuno Das Neves <nudasnev@microsoft.com>
@NunoDasNeves NunoDasNeves force-pushed the nudasnev/generic-hypercall-ioctl branch from 9ead785 to c03ec9c Compare April 23, 2024 19:52
@NunoDasNeves NunoDasNeves merged commit bea14dd into main Apr 23, 2024
2 checks passed
@NunoDasNeves NunoDasNeves deleted the nudasnev/generic-hypercall-ioctl branch April 23, 2024 22:02
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