rust-vmm / community

rust-vmm community content
502 stars 28 forks source link

Crate Addition Request: arch #41

Open jennymankin opened 5 years ago

jennymankin commented 5 years ago

Crate Name

arch (currently) or vmm-arch (to relate back to the rust-vmm project)

Short Description

A crate to provide a hypervisor-agnostic interface to the existing arch crate currently used by crosvm/Firecracker, and parts of which are used in libwhp.

Why is this crate relevant to the rust-vmm project?

The functionality provided by the arch crate is shared across multiple projects (Firecracker and crosvm), but currently uses KVM primitives and APIs directly. libwhp (using Hyper-V) uses a small portion of the logic but ports it to Hyper-V. A hypervisor-agnostic solution would allow the same crate to be used across projects regardless of hypervisor.

Design

The proposed arch crate relies on the abstraction of the VCPU to achieve hypervisor-agnosticism without sacrificing performance due to hypervisor-specific primitives.

When a hypervisor-agnostic arch crate was was initially proposed in the rust-vmm biweekly meeting, there was some concern expressed about actually losing KVM primitives in the abstraction, which could result in an unacceptable performance loss for Firecracker/crosvm. In practice, the KVM-specific primitives in the existing crate rely on direct operations on the KVM VcpuFd. Our design allows the continued use of these specific primitives by abstracting out the Vcpu functionality (as proposed in [link to Vcpu proposal]), altering the APIs to accept the Vcpu trait generic as an input parameter instead of the directly taking the data structure. And with Rust's compilation performing static dispatch, the abstraction has "zero cost".

Proposed Vcpu trait definition (proposed here):

pub trait Vcpu {
    fn set_fpu(&self, fpu: &Fpu) -> Result<()>;
    fn set_msrs(&self, msrs: &MsrEntries) -> Result<()>;
    // . . .
}

Example function from the existing arch crate

(Taking a VcpuFd reference as an input parameter):

pub fn setup_fpu(vcpu: &VcpuFd) -> Result<()> {
    let fpu: kvm_fpu = kvm_fpu {
        fcw: 0x37f,
        mxcsr: 0x1f80,
        ..Default::default()
    };

    vcpu.set_fpu(&fpu).map_err(Error::SetFPURegisters)
}

Refactor of function consuming the trait as a generic:

pub fn setup_fpu<T: Vcpu>(vcpu: &T) -> Result<()> {
    let fpu: Fpu = Fpu {
        fcw: 0x37f,
        mxcsr: 0x1f80,
        ..Default::default()
    };
    vcpu.set_fpu(&fpu).map_err(Error::SetFPURegisters)?;
    Ok(())
}

And code calling the arch function just calls it as normal, minimizing refactoring:

arch::x86_64::regs::setup_fpu(&self.fd).map_err(Error::FPUConfiguration)?;
zachreizner commented 5 years ago

I think this looks pretty good. Those that wish for dynamic dispatch can use the dynamic trait &Vcpu but those that want to choose at compile time to take advantage of static dispatch would simply use a concrete impl of Vcpu.