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: vm-vcpu-ref #121

Open
andreeaflorescu opened this issue Oct 27, 2021 · 6 comments
Open

Crate Addition Request: vm-vcpu-ref #121

andreeaflorescu opened this issue Oct 27, 2021 · 6 comments

Comments

@andreeaflorescu
Copy link
Member

Crate Name: vm-vcpu-ref

Short Description

This crate provides the primitives needed for setting up a VM for booting Linux. These include:

  • a basic filtering for cpuid
  • setting up the MP Table
  • setting up the GDT
  • setting up the registers & interrupts that are specific to x86_64 and aarch64
  • KvmVm and KvmVcpu structures that can be either used directly in other VMMs, or otherwise can serve as an example for setting up the VM for booting.

Initially, we propose to only add the KVM abstractions, but these can later be gated by a feature, and we can add support for other hypervisors.

We also propose to upstream these abstractions as part of vmm-reference as opposed to creating a separate repository as it is critical to test the integration of these abstractions in a VMM, and we already have the vmm-reference setup available.

Another important thing to note is that from this initial crate we can split multiple ones as the functionality matures. One good example is setting up the GDT. Right now this is done in a simplistic way, with just a few functions, and quite a few magic numbers and functionality that can be abstracted in a way that makes it easier to use. One example that comes to mind is creating the flags corresponding to a descriptor segment. Right now, we're just writing magic values like 0xa09b, 0xc093. What we could do instead is having a helper Flags where we can set up the various aspects in a human readable way, offering support through functions for setting up the privilege level, executable bit, granularity flag and so on.

Once we agree on a set of Vm, Vcpu and Hypervisor traits, these are going to also be implemented by the vm-vcpu-ref crate.

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

This crate reduces the duplication across existing products in terms of preparing a VM for booting. This is just one example of doing things, and multiple solutions can co-exist.

@andreeaflorescu
Copy link
Member Author

CC: @jiangliu @sboeuf @rbradford @sameo do you have anything against creating this crate? I'd like to start the upstreaming process, as I already have the patches available.
I am planning to do it in the following order:

  • setup for the GDT
  • basic cpuid
  • setup for the legacy interrupts
  • setup for the MP table (this needs to be extended to support PCI and interrupts)
  • setup for MSRs
  • GIC v2 & GIC v3 for arm
  • the Vm configuration that allows you to configure various aspects of the previously mentioned modules
  • the KvmVm & KvmVcpu abstractions that are setting up everything needed for booting.

I've created the crate as part of vmm-reference. The PR is available here: rust-vmm/vmm-reference#182. I've already prepared GDT and the basic CPUID, and they're available in my POC branch here: https://github.com/andreeaflorescu/vmm-reference/tree/arm_poc. This is getting a bit hard to maintain, and I would really appreciate you taking a look at the proposal here so I can start the upstreaming process. We can discuss more about the design on the PRs themselves.

@jiangliu
Copy link
Member

jiangliu commented Nov 1, 2021

I still like the original abstraction of "arch" or "vm-arch". The proposed crate may covers too much vmm specific logic, which may make it hard for reuse.

@andreeaflorescu
Copy link
Member Author

We can discuss about splitting this in 2 crates:

  • one that handles the gdt, cpuid, gic and so on
  • one that exports the KvmVm & KvmVcpu abstractions

We would still like to have an independent vm-vcpu-ref (or another name that is more appropriate), that can provide a default and extendable implementation of setting up the vcpu & vm for booting. This would not be a leaf crate (i.e. it will not be used as a dependency in other rust-vmm crates), and using it is optional. I was thinking that there is still value in having it in rust-vmm though as a means of showing how to use kvm-ioctls, bindings, and the other modules (i.e. gdt, mptable) to set up a minimal vcpu for booting.

I don't particularly like the idea of having something called "arch" because "arch" is not a component, it is just a name that we're using for things that we want to artificially group together. These things are used for setting up the VM & vCPU for booting, and this is why we are thinking about making them modules of vm-vcpu-ref.

What do you mean by VMM specific logic in this case? The idea is to have it independent of the product specific code. For this to happen we would need to first identify what is product specific. A few things that we already discussed about:

  • setting up the interrupts
  • using mptable vs acpi

These can be addressed through the configurations (i.e VmConfig and VcpuConfig). Are there any other things that you think are product specific in this case?

@jiangliu
Copy link
Member

jiangliu commented Nov 1, 2021

We can discuss about splitting this in 2 crates:

  • one that handles the gdt, cpuid, gic and so on
  • one that exports the KvmVm & KvmVcpu abstractions

We would still like to have an independent vm-vcpu-ref (or another name that is more appropriate), that can provide a default and extendable implementation of setting up the vcpu & vm for booting. This would not be a leaf crate (i.e. it will not be used as a dependency in other rust-vmm crates), and using it is optional. I was thinking that there is still value in having it in rust-vmm though as a means of showing how to use kvm-ioctls, bindings, and the other modules (i.e. gdt, mptable) to set up a minimal vcpu for booting.

I don't particularly like the idea of having something called "arch" because "arch" is not a component, it is just a name that we're using for things that we want to artificially group together. These things are used for setting up the VM & vCPU for booting, and this is why we are thinking about making them modules of vm-vcpu-ref.

I'm used to Linux source code structure:)
"gdt, cpuid, gic " should be common enough, but I'm not sure whether vm/vcpu is in the same situation, especially there are multiple hypervisors (kvm, hyperv, or maybe Xen). To be honest, I have read code related to HyperV yet.

What do you mean by VMM specific logic in this case? The idea is to have it independent of the product specific code. For this to happen we would need to first identify what is product specific. A few things that we already discussed about:

  • setting up the interrupts
  • using mptable vs acpi

These can be addressed through the configurations (i.e VmConfig and VcpuConfig). Are there any other things that you think are product specific in this case?

Maybe I misread your proposal. The way to configure vm/vcpu should be common enough, but the way to control vCPU may be different among VMMs. I have glanced one version of vm-vcpu-ref, which contains code to control vCPUs.

@andreeaflorescu
Copy link
Member Author

Maybe I misread your proposal. The way to configure vm/vcpu should be common enough, but the way to control vCPU may be different among VMMs. I have glanced one version of vm-vcpu-ref, which contains code to control vCPUs.

Oh yeah, I totally agree with you, handling the run of vcpus is very much product specific. That will not be included in the initial crate. I was working on abstracting that part away (and so far I miserably failed 😆 which is why I am not even considering running the vcpus to be part of the crate). There are just too many things that are product specific that need to happen before running the vcpu loop, and even the vcpu exits are product specific. I am sorry I did not made this more obvious from the beginning.

@andreeaflorescu
Copy link
Member Author

andreeaflorescu commented Nov 1, 2021

The code that is currently in vm-vcpu-ref is not the code that we want to upstream. There are several improvements needed, and not all the functionality there is actually to be upstreamed. Parts of it will stay as product specific code.

So far, what I actually worked on to have it in an upstreamable state:

  • cpuid
  • gdt
  • and now mptable is in progress

All the other things should be ignored.

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