Add KVM_SET_USER_MEMORY_REGION2 and KVM_CREATE_GUEST_MEMFD ioctls

[MatiasVara]

Summary of the PR

Add support for KVM_SET_USER_MEMORY_REGION2 and KVM_CREATE_GUEST_MEMFD ioctls and their structures (see #262). This PR requires to update kvm-bidings so I keep it as draft until then.

Requirements

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

• [ ] All commits in this PR have Signed-Off-By trailers (with git commit -s), and the commit message has max 60 characters for the summary and max 75 characters for each description line.
• [ ] All added/changed functionality has a corresponding unit/integration test.
• [ ] All added/changed public-facing functionality has entries in the "Upcoming Release" section of CHANGELOG.md (if no such section exists, please create one).
• [ ] Any newly added unsafe code is properly documented.

[roypat]

Hey, thanks for this! I realize it's still a draft, but could you also add KVM_SET_MEMORY_ATTRIBUTES support as part of this? I think it's required to be able to make use of guest_memfd.

[MatiasVara]

Hey, thanks for this! I realize it's still a draft, but could you also add KVM_SET_MEMORY_ATTRIBUTES support as part of this? I think it's required to be able to make use of guest_memfd.

Yes, I think I can add it.

[roypat]

Hi @MatiasVara, sorry for the late response here, but if you want to get this across the finish line, you could do a PR on kvm-bindings asking for a new release with the 6.9 bindings, so that this can pass CI :)

[MatiasVara]

Hi @MatiasVara, sorry for the late response here, but if you want to get this across the finish line, you could do a PR on kvm-bindings asking for a new release with the 6.9 bindings, so that this can pass CI :)

Hello @roypat , do you mean to open an issue in kvm-binding asking for a new release? Or, submitting a PR with the required changes?

[roypat]

Hi @MatiasVara, sorry for the late response here, but if you want to get this across the finish line, you could do a PR on kvm-bindings asking for a new release with the 6.9 bindings, so that this can pass CI :)

Hello @roypat , do you mean to open an issue in kvm-binding asking for a new release? Or, submitting a PR with the required changes?

Opening a PR with the required changes, similar to https://github.com/rust-vmm/kvm-bindings/pull/103. We don't really have a set release schedule, so releases are mostly driven by contributors wanting to consume new changes and doing release PRs

[MatiasVara]

Hi @MatiasVara, sorry for the late response here, but if you want to get this across the finish line, you could do a PR on kvm-bindings asking for a new release with the 6.9 bindings, so that this can pass CI :)

Hello @roypat , do you mean to open an issue in kvm-binding asking for a new release? Or, submitting a PR with the required changes?

Opening a PR with the required changes, similar to rust-vmm/kvm-bindings#103. We don't really have a set release schedule, so releases are mostly driven by contributors wanting to consume new changes and doing release PRs

Oh, I see, so the PR does not include the actual changes on the bindings but just the bump of the release, right?

[roypat]

Hi @MatiasVara, sorry for the late response here, but if you want to get this across the finish line, you could do a PR on kvm-bindings asking for a new release with the 6.9 bindings, so that this can pass CI :)

Hello @roypat , do you mean to open an issue in kvm-binding asking for a new release? Or, submitting a PR with the required changes?

Opening a PR with the required changes, similar to rust-vmm/kvm-bindings#103. We don't really have a set release schedule, so releases are mostly driven by contributors wanting to consume new changes and doing release PRs

Oh, I see, so the PR does not include the actual changes on the bindings but just the bump of the release, right?

Yup, we merged an bindings updated in https://github.com/rust-vmm/kvm-bindings/pull/108, so all the required changes are already in main

[MatiasVara]

@roypat I just submitted the PR at https://github.com/rust-vmm/kvm-bindings/pull/111

[roypat]

@roypat I just submitted the PR at rust-vmm/kvm-bindings#111

We've merged the kvm-bindings PR and released 0.9.0 to crates.io :)

[MatiasVara]

Mh, I'm not actually sure what the correct cfgs here should be. Currently, CONFIG_KVM_PRIVATE_MEMORY (which is the thing that enables the CREATE_GUEST_MEMFD ioctl) is only enabled by KVM_CONFIG_X86_SW_PROTECTED_VM, e.g. we can only get a kernel that exposes the ioctl on x86_64. I'm pretty sure that we'll never get 32 bit support for it, so we should definitely remove the target_arch = "x86" directives (and rust-vmm also doesn't support 32 bit targets, iirc). As for ARM, I would expect that guest_memfd will be available eventually, so do we want to expose it in kvm-ioctls already, or only expose it once it actually becomes usable?

Thanks for the sugestions. I am working on aarch64 but I am OK to, for the moment, only expose the ioctl on x86_64. Is it not already available in arm?

[MatiasVara]

@roypat I just submitted the PR at rust-vmm/kvm-bindings#111

We've merged the kvm-bindings PR and released 0.9.0 to crates.io :)

Should I bump the dependency to kvm-bindings in another commit?

[jpbrucker]

aarch64 will eventually implement these capabilities since both CCA and pKVM plan to use it, but upstreaming the KVM support may be months or years away. 32-bit arm definitely won't implement them, because support for KVM was dropped a while ago.

In my opinion enabling this for aarch64 now should be fine, because userspace must check whether the APIs are available with KVM_CHECK_EXTENSION on KVM_CAP_USER_MEMORY2 and KVM_CAP_GUEST_MEMFD. So upstream arm64 will simply return 0 now, and 1 once the capability is upstreamed. Actually using the ioctls without first checking the extension should return an error at the moment.

[roypat]

Yeah, that makes sense to me, let's expose it on both aarch64 and x86_64 then :)

[roypat]

@roypat I just submitted the PR at rust-vmm/kvm-bindings#111

We've merged the kvm-bindings PR and released 0.9.0 to crates.io :)

Should I bump the dependency to kvm-bindings in another commit?

Yeah, once we do that the CI should also start giving us some meaningful results :)

[MatiasVara]

@roypat any idea why the CI is still failing?

[roypat]

@roypat any idea why the CI is still failing?

I'm assuming somewhere in KVM a field was changed to an anonymous union, and since Rust doesnt have anonymous unions, bindgen generated a named one. Just applying the compiler's suggestion should fix it

[roypat]

mh, right, our CI is using Ubuntu 22, which comes with 6.5, e.g. doesn't have support for guest_memfd yet. We could upgrade to ubuntu 24, which is 6.9 based, but the kernel would still need to be compiled with KVM_X86_SW_PROTECTED_VM to be able to run the tests, and even then they would fail on ARM because it doesn't have an equivalent of KVM_[x86_]SW_PROTECTED_VM. I guess for now it'll be fine to either drop the unit tests, or short circuit them based on a KVM capabilities check.

[MatiasVara]

mh, right, our CI is using Ubuntu 22, which comes with 6.5, e.g. doesn't have support for guest_memfd yet. We could upgrade to ubuntu 24, which is 6.9 based, but the kernel would still need to be compiled with KVM_X86_SW_PROTECTED_VM to be able to run the tests, and even then they would fail on ARM because it doesn't have an equivalent of KVM_[x86_]SW_PROTECTED_VM. I guess for now it'll be fine to either drop the unit tests, or short circuit them based on a KVM capabilities check.

Do you mean something like?

let mut config = kvm_enable_cap {
    cap: KVM_CAP_GUEST_MEMFD,
    ..Default::default()
};
if vm.enable_cap(&config).is_err() {
    return;
}

[roypat]

mh, right, our CI is using Ubuntu 22, which comes with 6.5, e.g. doesn't have support for guest_memfd yet. We could upgrade to ubuntu 24, which is 6.9 based, but the kernel would still need to be compiled with KVM_X86_SW_PROTECTED_VM to be able to run the tests, and even then they would fail on ARM because it doesn't have an equivalent of KVM_[x86_]SW_PROTECTED_VM. I guess for now it'll be fine to either drop the unit tests, or short circuit them based on a KVM capabilities check.

Do you mean something like?

let mut config = kvm_enable_cap {
    cap: KVM_CAP_GUEST_MEMFD,
    ..Default::default()
};
if vm.enable_cap(&config).is_err() {
    return;
}

Yeah

[MatiasVara]

mh, right, our CI is using Ubuntu 22, which comes with 6.5, e.g. doesn't have support for guest_memfd yet. We could upgrade to ubuntu 24, which is 6.9 based, but the kernel would still need to be compiled with KVM_X86_SW_PROTECTED_VM to be able to run the tests, and even then they would fail on ARM because it doesn't have an equivalent of KVM_[x86_]SW_PROTECTED_VM. I guess for now it'll be fine to either drop the unit tests, or short circuit them based on a KVM capabilities check.

Do you mean something like?

let mut config = kvm_enable_cap {
    cap: KVM_CAP_GUEST_MEMFD,
    ..Default::default()
};
if vm.enable_cap(&config).is_err() {
    return;
}

Yeah

I do not know if I fixed correctly. I make the examples compile only in x86-64 in which I can check the cap.

[roypat]

mh, right, our CI is using Ubuntu 22, which comes with 6.5, e.g. doesn't have support for guest_memfd yet. We could upgrade to ubuntu 24, which is 6.9 based, but the kernel would still need to be compiled with KVM_X86_SW_PROTECTED_VM to be able to run the tests, and even then they would fail on ARM because it doesn't have an equivalent of KVM_[x86_]SW_PROTECTED_VM. I guess for now it'll be fine to either drop the unit tests, or short circuit them based on a KVM capabilities check.

Do you mean something like?

let mut config = kvm_enable_cap {
    cap: KVM_CAP_GUEST_MEMFD,
    ..Default::default()
};
if vm.enable_cap(&config).is_err() {
    return;
}

Yeah

I do not know if I fixed correctly. I make the examples compile only in x86-64 in which I can check the cap.

Yeah, that works. Can you use https://doc.rust-lang.org/rustdoc/write-documentation/documentation-tests.html#hiding-portions-of-the-example to hide the cfg from docs.rs (might be easier if they're in the form of if !cfg!(target_arch = "x86") { return; })? Other than that, everything looks good to me!

[MatiasVara]

mh, right, our CI is using Ubuntu 22, which comes with 6.5, e.g. doesn't have support for guest_memfd yet. We could upgrade to ubuntu 24, which is 6.9 based, but the kernel would still need to be compiled with KVM_X86_SW_PROTECTED_VM to be able to run the tests, and even then they would fail on ARM because it doesn't have an equivalent of KVM_[x86_]SW_PROTECTED_VM. I guess for now it'll be fine to either drop the unit tests, or short circuit them based on a KVM capabilities check.

Do you mean something like?

let mut config = kvm_enable_cap {
    cap: KVM_CAP_GUEST_MEMFD,
    ..Default::default()
};
if vm.enable_cap(&config).is_err() {
    return;
}

Yeah

I do not know if I fixed correctly. I make the examples compile only in x86-64 in which I can check the cap.

Yeah, that works. Can you use https://doc.rust-lang.org/rustdoc/write-documentation/documentation-tests.html#hiding-portions-of-the-example to hide the cfg from docs.rs (might be easier if they're in the form of if !cfg!(target_arch = "x86") { return; })? Other than that, everything looks good to me!

I added # to hide the cfg from docs.rs. I could not make work if !cfg!(target_arch = "x86") { return; } locally. I am probably missing something.

[MatiasVara]

Thanks! Sorry the review dragged on for so long

That's alright, I learned a lot.