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

Add support for KVM_ENABLE_CAP ioctl #49

Merged
merged 1 commit into from
Jun 12, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/cap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,5 +132,7 @@ pub enum Cap {
PpcEnableHcall = KVM_CAP_PPC_ENABLE_HCALL,
CheckExtensionVm = KVM_CAP_CHECK_EXTENSION_VM,
S390UserSigp = KVM_CAP_S390_USER_SIGP,
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
SplitIrqchip = KVM_CAP_SPLIT_IRQCHIP,
ImmediateExit = KVM_CAP_IMMEDIATE_EXIT,
}
89 changes: 89 additions & 0 deletions src/ioctls/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -610,6 +610,62 @@ impl VmFd {
Ok(())
}

/// Enable the specified capability as per the `KVM_ENABLE_CAP` ioctl.
///
/// See the documentation for `KVM_ENABLE_CAP`.
///
/// Returns an io::Error when the capability could not be enabled.
///
/// # Arguments
///
/// * kvm_enable_cap - KVM capability structure. For details check the `kvm_enable_cap`
/// structure in the
/// [KVM API doc](https://www.kernel.org/doc/Documentation/virtual/kvm/api.txt).
///
/// # Example
///
/// ```rust
/// # extern crate kvm_ioctls;
Copy link
Member

Choose a reason for hiding this comment

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

Please remove his line (and the next one prefixed by #). They don't show up in the generated doc and the doctest passes without them.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed but those lines (prefixed by #) where there in every other doctest.

Copy link
Author

@sboeuf sboeuf Jun 12, 2019

Choose a reason for hiding this comment

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

I can't remove it, the CI complains:


---- src/ioctls/vm.rs - ioctls::vm::VmFd::enable_cap (line 627) stdout ----
error[E0433]: failed to resolve: use of undeclared type or module `Kvm`
--> src/ioctls/vm.rs:632:11
 |
7 | let kvm = Kvm::new().unwrap();
 | ^^^ use of undeclared type or module `Kvm`

/// extern crate kvm_bindings;
///
/// # use kvm_ioctls::{Cap, Kvm, VmFd};
/// use kvm_bindings::{kvm_enable_cap, KVM_CAP_SPLIT_IRQCHIP};
///
/// let kvm = Kvm::new().unwrap();
/// let vm = kvm.create_vm().unwrap();
/// let mut cap: kvm_enable_cap = Default::default();
/// // This example cannot enable an arm/aarch64 capability since there
/// // is no capability available for these architectures.
/// if cfg!(target_arch = "x86") || cfg!(target_arch = "x86_64") {
/// cap.cap = KVM_CAP_SPLIT_IRQCHIP;
/// // As per the KVM documentation, KVM_CAP_SPLIT_IRQCHIP only emulates
/// // the local APIC in kernel, expecting that a userspace IOAPIC will
/// // be implemented by the VMM.
/// // Along with this capability, the user needs to specify the number
/// // of pins reserved for the userspace IOAPIC. This number needs to be
/// // provided through the first argument of the capability structure, as
/// // specified in KVM documentation:
/// // args[0] - number of routes reserved for userspace IOAPICs
/// //
/// // Because an IOAPIC supports 24 pins, that's the reason why this test
/// // picked this number as reference.
/// cap.args[0] = 24;
/// vm.enable_cap(&cap).unwrap();
/// }
/// ```
///
#[cfg(not(any(target_arch = "arm", target_arch = "aarch64")))]
pub fn enable_cap(&self, cap: &kvm_enable_cap) -> Result<()> {
Copy link
Member

Choose a reason for hiding this comment

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

What architectures is this available on? The documentation only mentions mips, ppc, s390, I found patches for x86, but what about aarch64?

Copy link
Author

Choose a reason for hiding this comment

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

The documentation states that all architectures support this ioctl:

4.37 KVM_ENABLE_CAP

Capability: KVM_CAP_ENABLE_CAP
Architectures: mips, ppc, s390
Type: vcpu ioctl
Parameters: struct kvm_enable_cap (in)
Returns: 0 on success; -1 on error

Capability: KVM_CAP_ENABLE_CAP_VM
Architectures: all
Type: vcpu ioctl
Parameters: struct kvm_enable_cap (in)
Returns: 0 on success; -1 on error

+Not all extensions are enabled by default. Using this ioctl the application
can enable an extension, making it available to the guest.

On systems that do not support this ioctl, it always fails. On systems that
do support it, it only works for extensions that are supported for enablement.

To check if a capability can be enabled, the KVM_CHECK_EXTENSION ioctl should
be used.

struct kvm_enable_cap {
       /* in */
       __u32 cap;

The capability that is supposed to get enabled.

       __u32 flags;

A bitfield indicating future enhancements. Has to be 0 for now.

       __u64 args[4];

Arguments for enabling a feature. If a feature needs initial values to
function properly, this is the place to put them.

       __u8  pad[64];
};

The vcpu ioctl should be used for vcpu-specific capabilities, the vm ioctl
for vm-wide capabilities.

Copy link
Author

Choose a reason for hiding this comment

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

Oh my bad! Talking with @andreeaflorescu I realized this snippet of the doc is not part of 4.14 or 4.20, it's only been introduced later in 5.x.
So I suggest that for now I can remove the arm/aarch64 target.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good to me.

Copy link
Author

Choose a reason for hiding this comment

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

Ok I've updated the PR!

// The ioctl is safe because we allocated the struct and we know the
// kernel will write exactly the size of the struct.
let ret = unsafe { ioctl_with_ref(self, KVM_ENABLE_CAP(), cap) };
if ret == 0 {
Ok(())
} else {
Err(io::Error::last_os_error())
}
}

/// Get the `kvm_run` size.
pub fn run_size(&self) -> usize {
self.run_size
Expand Down Expand Up @@ -812,4 +868,37 @@ mod tests {
let msi = kvm_msi::default();
assert!(vm.signal_msi(msi).is_err());
}

#[test]
#[cfg(not(any(target_arch = "arm", target_arch = "aarch64")))]
fn test_enable_cap_failure() {
let kvm = Kvm::new().unwrap();
let vm = kvm.create_vm().unwrap();
let cap: kvm_enable_cap = Default::default();
// Providing the `kvm_enable_cap` structure filled with default() should
// always result in a failure as it is not a valid capability.
assert!(vm.enable_cap(&cap).is_err());
}

#[test]
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
fn test_enable_split_irqchip_cap() {
sboeuf marked this conversation as resolved.
Show resolved Hide resolved
let kvm = Kvm::new().unwrap();
let vm = kvm.create_vm().unwrap();
let mut cap: kvm_enable_cap = Default::default();
cap.cap = KVM_CAP_SPLIT_IRQCHIP;
// As per the KVM documentation, KVM_CAP_SPLIT_IRQCHIP only emulates
// the local APIC in kernel, expecting that a userspace IOAPIC will
// be implemented by the VMM.
// Along with this capability, the user needs to specify the number
// of pins reserved for the userspace IOAPIC. This number needs to be
// provided through the first argument of the capability structure, as
// specified in KVM documentation:
// args[0] - number of routes reserved for userspace IOAPICs
//
// Because an IOAPIC supports 24 pins, that's the reason why this test
// picked this number as reference.
cap.args[0] = 24;
assert!(vm.enable_cap(&cap).is_ok());
}
}
2 changes: 2 additions & 0 deletions src/kvm_ioctls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ ioctl_iow_nr!(KVM_SET_FPU, KVMIO, 0x8d, kvm_fpu);
ioctl_ior_nr!(KVM_GET_LAPIC, KVMIO, 0x8e, kvm_lapic_state);
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
ioctl_iow_nr!(KVM_SET_LAPIC, KVMIO, 0x8f, kvm_lapic_state);
#[cfg(not(any(target_arch = "arm", target_arch = "aarch64")))]
ioctl_iow_nr!(KVM_ENABLE_CAP, KVMIO, 0xa3, kvm_enable_cap);
#[cfg(any(
target_arch = "x86",
target_arch = "x86_64",
Expand Down
2 changes: 1 addition & 1 deletion tests/coverage
Original file line number Diff line number Diff line change
@@ -1 +1 @@
91.2
91.3