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

Crate Addition Request: arch #41

Open
jennymankin opened this issue Mar 18, 2019 · 1 comment
Open

Crate Addition Request: arch #41

jennymankin opened this issue Mar 18, 2019 · 1 comment

Comments

@jennymankin
Copy link

jennymankin commented Mar 18, 2019

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
Copy link
Member

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.

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

No branches or pull requests

2 participants