Add support for KVM_MEMORY_ENCRYPT_{OP,REG_REGION,UNREG_REGION}

[rvolosatovs]

Add support for 3 ioctls:

• KVM_MEMORY_ENCRYPT_OP
• KVM_MEMORY_ENCRYPT_REG_REGION (SEV-specific)
• KVM_MEMORY_ENCRYPT_UNREG_REGION (SEV-specific)

cc @haraldh

[rvolosatovs]

>           assert False, "Coverage drops by {:.2f}%. Please add unit tests for " \
                          "the uncovered lines.".format(diff)
E           AssertionError: Coverage drops by 2.00%. Please add unit tests for the uncovered lines.
E           assert False

This is tricky, since the functionality can only run on SEV-enabled hosts. I guess it would be possible to add a feature flag and guard this functionality and respective tests, although not sure if that's really necessary at this point.

[rvolosatovs]

Encountered some issues while testing, marking as draft until I'm sure this works as expected. that turned out to be initialization issue, SEV has to be enabled first

[rvolosatovs]

Made names shorter, while staying consistent with e.g. https://docs.rs/kvm-ioctls/0.10.0/kvm_ioctls/struct.VmFd.html#method.set_user_memory_region:

diff --git a/src/ioctls/vm.rs b/src/ioctls/vm.rs
index ea97cbc..d7faf6d 100644
--- a/src/ioctls/vm.rs
+++ b/src/ioctls/vm.rs
@@ -1283,11 +1283,11 @@ impl VmFd {
     ///     addr: 0x10000 as u64,
     ///     size: 0x10000 as u64,
     /// };
-    /// vm.register_encrypted_memory_region(&memory_region).unwrap();
+    /// vm.register_enc_memory_region(&memory_region).unwrap();
     /// ```
     ///
     #[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
-    pub fn register_encrypted_memory_region(&self, memory_region: &kvm_enc_region) -> Result<()> {
+    pub fn register_enc_memory_region(&self, memory_region: &kvm_enc_region) -> Result<()> {
         // Safe because we know that our file is a VM fd, we know the kernel will only read the
         // correct amount of memory from our pointer, and we verify the return result.
         let ret = unsafe { ioctl_with_ref(self, KVM_MEMORY_ENCRYPT_REG_REGION(), memory_region) };
@@ -1321,11 +1321,11 @@ impl VmFd {
     ///     addr: 0x10000 as u64,
     ///     size: 0x10000 as u64,
     /// };
-    /// vm.unregister_encrypted_memory_region(&memory_region).unwrap();
+    /// vm.unregister_enc_memory_region(&memory_region).unwrap();
     /// ```
     ///
     #[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
-    pub fn unregister_encrypted_memory_region(&self, memory_region: &kvm_enc_region) -> Result<()> {
+    pub fn unregister_enc_memory_region(&self, memory_region: &kvm_enc_region) -> Result<()> {
         // Safe because we know that our file is a VM fd, we know the kernel will only read the
         // correct amount of memory from our pointer, and we verify the return result.
         let ret = unsafe { ioctl_with_ref(self, KVM_MEMORY_ENCRYPT_UNREG_REGION(), memory_region) };

[lauralt]

You'll need to also update the coverage score so that the all the tests pass.

It looks like there's an issue with CI (perhaps due to the config attribute?), since all the added functionality should be covered with tests at this point and there are also examples for all functionality, which are tested on supported systems

kcov is counting the lines that are executed by tests on the platform on which it runs, the CI instance doesn't have sev enabled, so the sev functionality covered by your tests won't be executed when running kcov. We are not yet running the CI on AMD, so until that point we can decrease the coverage value :(.

[andreeaflorescu]

It looks like there's an issue with CI (perhaps due to the config attribute?), since all the added functionality should be covered with tests at this point and there are also examples for all functionality, which are tested on supported systems

The host on which the CI runs does not have SEV enabled. This means that the tests with #[cfg_attr(not(has_sev), ignore)] are ignored, so they do not contribute to the coverage score. You should just update the coverage score, and we can work on having a host with SEV so we can make sure that the tests are running in the CI as well.

LE: Laura already replied to this.

[jiangliu]

Is there plan to add more for KVM sev commands such as KVM_SEV_LAUNCH_START, KVM_SEV_LAUNCH_UPDATE_DATA?
I'm thinking whether we should introduce submodules for sev/tdx etc.

[rvolosatovs]

Is there plan to add more for KVM sev commands such as KVM_SEV_LAUNCH_START, KVM_SEV_LAUNCH_UPDATE_DATA? I'm thinking whether we should introduce submodules for sev/tdx etc.

AFAIK there was an (unsuccessful?) attempt to add SEV commands in #111.

We currently maintain most of SEV-specific functionality externally, but if there is interest here we may consider contributing those.

[jiangliu]

#111

The enarx project also makes use of the kvm-bindings/kvm-ioctls crates, so is ti feasible to move some needed data structures from enarx into kvm-ioctls? But I do have some concern about openssl, which is always troublesome.

[rvolosatovs]

#111

The enarx project also makes use of the kvm-bindings/kvm-ioctls crates, so is ti feasible to move some needed data structures from enarx into kvm-ioctls? But I do have some concern about openssl, which is always troublesome.

We are actually deprecating SEV crate at the moment, so it could be a good chance to salvage useful parts.
openssl is only necessary for certificate stuff, which not necessarily has to be added here, I believe
We could file an issue about this and discuss there?

[jiangliu]

#111

The enarx project also makes use of the kvm-bindings/kvm-ioctls crates, so is ti feasible to move some needed data structures from enarx into kvm-ioctls? But I do have some concern about openssl, which is always troublesome.

We are actually deprecating SEV crate at the moment, so it could be a good chance to salvage useful parts. openssl is only necessary for certificate stuff, which not necessarily has to be added here, I believe We could file an issue about this and discuss there?

Yes, a good idea to open a new issue for discussion.

[rvolosatovs]

Any update on this?

[rvolosatovs]

• Added documentation and license header to build.rs
• Fixed outdated doc reference
• Ensured the doc reference format is consistent with Rust stdlib and represents actual method references
• Added test cases for invalid KVM_MEMORY_ENCRYPT_{,UN}REG_REGION usage (I suppose it does not make much sense to do that for encrypt_op, since it's platform-specific)
• Made documentation more consistent with existing methods on VmFd

[lauralt]

* Added documentation and license header to `build.rs`

* Fixed outdated doc reference

* Ensured the doc reference format is consistent with Rust stdlib and represents actual method references

* Added test cases for invalid `KVM_MEMORY_ENCRYPT_{,UN}REG_REGION` usage (I suppose it does not make much sense to do that for `encrypt_op`, since it's platform-specific)

* Made documentation more consistent with existing methods on `VmFd`

Thanks!