gnif / LookingGlass

An extremely low latency KVMFR (KVM FrameRelay) implementation for guests with VGA PCI Passthrough.
GNU General Public License v2.0
4.72k stars 260 forks source link

Note on AMD SEV support for kvmfr kernel module #1077

Closed ZenithalHourlyRate closed 1 year ago

ZenithalHourlyRate commented 1 year ago

This is kind of a bug report as the current kvmfr kernel module does not work in a VM when AMD SEV is enabled, but it seems that it is mostly not the use case here so I just post it as a note in case some other people encounter the same issue.

Some background, AMD SEV (Secure Encrypted Virtualization) will bring up a VM with its memory fully encrypted. kvmfr will "hotplug" the ivshmem to the VM as normal memory, so it will also be encrypted. However, once ivshmem is encrypted it is not shared anymore, so we need to clear the encryption bit of the PTE of the ivshmem.

diff --git a/module/kvmfr.c b/module/kvmfr.c
index ca0cca68..412e0b87 100644
--- a/module/kvmfr.c
+++ b/module/kvmfr.c
@@ -33,6 +33,10 @@

 #include <asm/io.h>

+#ifdef CONFIG_AMD_MEM_ENCRYPT
+#include <asm/mem_encrypt.h>
+#endif
+
 #include "kvmfr.h"

 DEFINE_MUTEX(minor_lock);
@@ -321,6 +325,25 @@ static int device_mmap(struct file * filp, struct vm_area_struct * vma)
   switch (kdev->type)
   {
     case KVMFR_TYPE_PCI:
+#ifdef CONFIG_AMD_MEM_ENCRYPT
+      /* Clear C-bit for ivshmem when mapped
+       * as normal mem to the userspace
+       *
+       * devm_memremap below will "hotplug" the ivshmem as normal mem
+       * and then sev and sev-snp will be effective
+       * and it will then be encrypted and private memory.
+       *
+       * However, this is not the intention of ivshmem, as it
+       * is meant to be shared with other VMs and the hypervisor.
+       *
+       * Mapping ivshmem as iomem could resolve the sev/sev-snp issue,
+       * but it then will not be cached and the performance is low.
+       *
+       * To maintain high performance yet make it shared, we should
+       * clear the C-bit for ivshmem.
+       */
+      vma->vm_page_prot.pgprot &= ~(sme_me_mask);
+#endif
       vma->vm_ops          = &pci_mmap_ops;
       vma->vm_private_data = kdev;
       return 0;
gnif commented 1 year ago

Make this a pull request and we will merge it in. I am running EPYC with SEV and can verify this. Be sure to bump the KVMFR version in the module and in the DKMS config, and add yourself to AUTHORS

ZenithalHourlyRate commented 1 year ago

An extra question: Have LookingGlass considered upstreaming the ivshmem pci driver part to the mainline? Such a device would be handy for many other use cases.

gnif commented 1 year ago

This has been raised many times, someone at one point even attempted to upstream it on our behalf. The reason we did not do this is that we knew that it would be rejected (and it was) as it largely duplicates existing functionality in the kernel already. We need this driver to work around an intentional security limitation in the uio implementation that prevents us using DMABUF.

ZenithalHourlyRate commented 1 year ago

we knew that it would be rejected (and it was)

Could a link to that patch thread be provided?

it largely duplicates existing functionality in the kernel already

I am curious which driver would do the similar thing

gnif commented 1 year ago

Could a link to that patch thread be provided?

If I can find it, it was quite some time ago

I am curious which driver would do the similar thing

udambuf, I wrote KVMFR largely based on this module, but removing the F_SEAL_SHRINK seal requirement on the file used. This seal was preventing us from using a shared memory file for a dmabuf as a shared memory file can be shrunk in size while in use. I proposed removing this restriction but it was rejected.