rust-vmm / kvm-ioctls

Apache License 2.0
270 stars 106 forks source link

Add support for coalesced MMIO (`KVM_CAP_COALESCED_MMIO` / `KVM_CAP_COALESCED_PIO`) #244

Closed 00xc closed 9 months ago

00xc commented 10 months ago

Summary of the PR

Add support for coalesced MMIO. This performance feature allows guest writes to port and memory space to not trigger VM exits. Instead, the kernel will write an entry into a shared ring buffer for each access, which userspace must consume. The ring buffer is mapped at a certain offset in the vcpu's file descriptor.

In order to enable this capability, introduce the KvmCoalescedIoRing struct, which will act as a safe wrapper around the raw mapping of the ring buffer. Since users may not want to use coalesced MMIO, or it might not be available, store it as an Option in the VcpuFd struct.

Add two tests as well for the newly added code.

The public API consists of:

Requirements

Before submitting your PR, please make sure you addressed the following requirements:

00xc commented 10 months ago

@JonathanWoollett-Light addressed your suggestions in the newer version, except the ones about using function mnemonics. Regarding that, isn't that change out of scope for this PR? It would add a new dependency and we would still have a bunch of tests which use raw bytes as assembly code. I'd rather work on that change in a separate PR.

00xc commented 10 months ago

By the way the code can also properly be assembled with that crate's CodeAssembler, e.g.

let mut asm = CodeAssembler::new(16).unwrap();
asm.out(0x2c, al).unwrap();
asm.hlt().unwrap();
asm.out(0x2c, al).unwrap();
asm.hlt().unwrap();
let code = asm.assemble(base_addr).unwrap();
JonathanWoollett-Light commented 10 months ago

@JonathanWoollett-Light addressed your suggestions in the newer version, except the ones about using function mnemonics. Regarding that, isn't that change out of scope for this PR? It would add a new dependency and we would still have a bunch of tests which use raw bytes as assembly code. I'd rather work on that change in a separate PR.

Yeah this makes sense.

00xc commented 10 months ago

Rebased on main and fixed conflicts

00xc commented 10 months ago

Gentle ping for reviewers :)

JonathanWoollett-Light commented 9 months ago

@00xc Can you please resolve the merge conflicts, after this LGTM.

00xc commented 9 months ago

Rebased on main and fixed conflicts