Open Stack

> The required changes are summarized below:
> > 	* Container kernel:
> > 		- Add SEV support patches from the Linux kernel repo in [3].
> > 		- Force virtio to use the DMA API (and hence SWIOTLB) when
> adding/removing buffers to/from the virtio ring buffer.
> >
> Ah, now I understand where the iommu question on the cc-devel mailing
> list was coming from :) Would you mind explaining why you need SWIOTLB
> when SEV is enabled? Also, I assume you need to force all virtio devices
> to do DMA, right? Did you just hack vring_use_dma_api() at the moment?
> 
> 

Ha ha! Yup, now you have the whole story. :) I never got around to thanking your for your reply in that thread. It was really helpful. Thanks!

On EPYC, our IOMMU doesn't yet support SEV, so DMA to/from devices needs to be done using unencrypted pages. It was easy to implement this using the bounce buffers provided by SWIOTLB. As you guessed, a quick change to vring_use_dma_api() got virtio support working properly:

amd at pecanporter:~/src/git/AMDSEV/src/kvm$ git diff
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index eb30f3e09a47..1bba0a6c1668 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -25,6 +25,7 @@
 #include <linux/hrtimer.h>
 #include <linux/kmemleak.h>
 #include <linux/dma-mapping.h>
+#include <linux/mem_encrypt.h>
 #include <xen/xen.h>

 #ifdef DEBUG
@@ -147,6 +148,9 @@ static bool vring_use_dma_api(struct virtio_device *vdev)
        if (!virtio_has_iommu_quirk(vdev))
                return true;

+       if (mem_encrypt_active())
+               return true;
+
        /* Otherwise, we are left to guess. */
        /*
         * In theory, it's possible to have a buggy QEMU-supposed

> > 		- SEV requires a memory copy in order to perform the
> encryption, so zero-copy solutions using DAX for the container initial user
> space will not work.
> > 			+ Build in a small initramfs to use as the guest kernel
> initial user space.
> > 			+ Include the updated container agent binary and
> supporting libs (~14MB total).
> So not using nvdimm from the QEMU command line and switching to a virtio
> block would have been sufficient here, right?
> 

Yes, virtio-blk should work for this as well. 

> > As a check, dumping the contents of a page from the qemu heap reveals
> plaintext data:
> >
> > amd at pecanporter:~/src/git$ sudo dd if=/proc/$(pgrep qemu)/mem
> bs=4096 count=1 skip=23058854513 | xxd | tail
> > dd: /proc/38572/mem: cannot skip to specified offset
> > 1+0 records in
> > 1+0 records out
> > 4096 bytes (4.1 kB, 4.0 KiB) copied, 8.8437e-05 s, 46.3 MB/s
> > 00000f60: 	0000 0000 0000 0000 0000 0000 0000 0000	................
> > 00000f70: 	0000 0000 0000 0000 0000 0000 0000 0000	................
> > 00000f80: 	0000 0000 0000 0000 7100 0000 0000 0000	........q.......
> > 00000f90: 	2f72 756e 2f76 6972 7463 6f6e 7461 696e
> 	/run/virtcontain
> > 00000fa0: 	6572 732f 706f 6473 2f33 3565 3233 6565
> 	ers/pods/35e23ee
> > 00000fb0: 	3330 6466 6237 3266 3135 3730 6265 3432
> 	30dfb72f1570be42
> > 00000fc0: 	6665 3165 6331 3366 3331 3332 6138 6133
> 	fe1ec13f3132a8a3
> > 00000fd0: 	6463 3336 6463 3131 6235 6365 3837 6236
> 	dc36dc11b5ce87b6
> > 00000fe0: 	3437 3930 3736 6339 612f 636f 6e73 6f6c
> 	479076c9a/consol
> > 00000ff0: 	652e 736f 636b 0000 0104 0000 0000 0000	e.sock..........
> >
> > However, any attempt to read the container memory from the host
> produces only ciphertext:
> >
> > amd at pecanporter:~/src/git$ sudo dd if=/proc/$(pgrep qemu)/mem
> bs=4096 count=1 skip=34165702144 | xxd | head
> > dd: /proc/38572/mem: cannot skip to specified offset
> > 1+0 records in
> > 1+0 records out
> > 4096 bytes (4.1 kB, 4.0 KiB) copied, 8.9039e-05 s, 46.0 MB/s
> > 00000000: 	e9b8 e14d c063 ee18 fd85 5ecc 4d1f c1a2
> 	...M.c....^.M...
> > 00000010: 	d681 cdf2 259b a97e c43b 5cde bf9e 695b	....%..~.;\...i[
> > 00000020: 	db3c 778b 8e77 89f4 f795 e5a6 9ebb 765b	.<w..w........v[
> > 00000030: 	0905 e1d3 c7ec 6f2b bada ed15 b2e0 db7f	......o+........
> > 00000040: 	d5e9 6d15 cf28 0ca1 4a45 3b9a 1779 e3ff	..m..(..JE;..y..
> > 00000050: 	9ee0 b562 2311 6e5a e972 4c06 3f6a 6ebf
> 	...b#.nZ.rL.?jn.
> > 00000060: 	909a 88ea 737a 6226 5d87 8968 b31b d096	....szb&]..h....
> > 00000070: 	9360 cbb0 4f34 d811 89a7 048f 01e8 d19e	.`..O4..........
> > 00000080: 	5429 995a 4de0 6fba 3360 8bb4 a2dc 17e4
> 	T).ZM.o.3`......
> > 00000090: 	80f5 6657 9fd7 0347 e78d 4d13 6b6c c649
> 	..fW...G..M.kl.I
> >
> Sweet!
> 

Thanks! I get a kick out of this too. :D

> 
> > Our threat model is to allow container workloads to reduce their risk
> exposure to security vulnerabilities in the hosting environment, which seems
> to overlap nicely with the threat model of Kata Containers. Is this a feature
> that the Kata community would find useful? If so, we would be very
> interested to work with the community to enable SEV memory encryption
> for Kata Containers. Any and all feedback is welcome!
> >
> So I guess we'll gather the kernel, qemu and firmware patches through
> upstream at some point. Or we can backport them once they're in if we
> don't want to move to the latest versions for those.
> I guess Kata Containers main task to support this would be at the
> hypervisor level, specifically at being able to pass the right options
> to QEMU. In my mind we should make our qemu hypervisor implementation
> detect SEV/MK-TME support dynamically and set the right qemu options
> (+ memory-encryption, - nvdimm) by default when the host CPU supports
> it. I believe we should also provide an opt-out runtime option for those
> who don't want to pay the performance penalty of memory encryption.
> 

Yes, the largest changes were to teach virtcontainers/govmm how to enable memory encryption in qemu. The Linux kernel currently has a boot parameter to disable memory encryption support (mem_encrypt=off), which could be added to the guest kernel "append" option in the config file, but that won't prevent qemu from creating the (unused) memory encryption machine objects. I agree that a proper "chicken bit" option in the config file would be appropriate.

> Cheers,
> Samuel.

Sincerely,
Jesse