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

Minor enhancement and bugfixes #151

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

Conversation

jiangliu
Copy link
Member

@jiangliu jiangliu commented Jun 5, 2021

Enhance the kvm-ioctls with:

  1. a new Kvm::get_disable_exits_cap() method
  2. a new VcpuFd::vcpu_file() access method
  3. export VmFd::new_vmfd()
  4. fix bugs in Kvm::get_max_vcpus() and Kvm::get_max_vcpu_id()

@jiangliu jiangliu force-pushed the enhance branch 6 times, most recently from 2fd74d8 to 4bb478d Compare June 5, 2021 13:16
@jiangliu
Copy link
Member Author

jiangliu commented Jun 5, 2021

The first patch is temporary workaround for #152

src/ioctls/system.rs Outdated Show resolved Hide resolved
src/ioctls/system.rs Outdated Show resolved Hide resolved
src/ioctls/system.rs Outdated Show resolved Hide resolved
src/ioctls/system.rs Outdated Show resolved Hide resolved
src/ioctls/system.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/ioctls/vcpu.rs Outdated Show resolved Hide resolved
@lauralt
Copy link
Collaborator

lauralt commented Jun 15, 2021

The CI machine was updated, so I think the first commit is not required anymore.

@jiangliu
Copy link
Member Author

The CI machine was updated, so I think the first commit is not required anymore.

Done

acatangiu
acatangiu previously approved these changes Nov 23, 2021
src/ioctls/vcpu.rs Outdated Show resolved Hide resolved
Add method to query KVM_CAP_X86_DISABLE_EXITS for x86 platforms.

Signed-off-by: Liu Jiang <[email protected]>
Kvm::check_extension_int() may return negative value as error code,
so Kvm::get_max_vcpus() and Kvm::get_max_vcpu_id() should handle the
error cases.

Signed-off-by: Liu Jiang <[email protected]>
/// Returns the "disable idle exiting" capability.
///
/// The `KVM_CAP_X86_DISABLE_EXITS` capability provides userspace with per-VM capability
/// to not intercept MWAIT/HLT/PAUSE/CSTATE, which means though instructions won't cause
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
/// to not intercept MWAIT/HLT/PAUSE/CSTATE, which means though instructions won't cause
/// to not intercept MWAIT/HLT/PAUSE/CSTATE, which means such instructions won't cause

or these

Comment on lines +636 to +637
assert!(disable_exits & KVM_X86_DISABLE_EXITS_HLT != 0);
assert!(disable_exits & KVM_X86_DISABLE_EXITS_PAUSE != 0);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Running this test on my laptop fails, should we check here for some capability before doing the asserts? Maybe this way we can also have a test case for get_disable_exits returning 0.

Comment on lines 261 to 265
if x > 0 {
x as u32
} else {
0
}
Copy link

Choose a reason for hiding this comment

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

This can be slightly simplified.

Suggested change
if x > 0 {
x as u32
} else {
0
}
u32::try_from(x).unwrap_or(0)

@andreeaflorescu
Copy link
Member

@jiangliu are you still interested in getting some of these changes merged?

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.

5 participants