On Mon, Oct 11, 2021 at 08:15:37PM -0500, Michael Roth wrote:
On Sun, Oct 10, 2021 at 08:17:00PM -0700, Marc Orr wrote:
On Wed, Oct 6, 2021 at 1:40 PM Michael Roth michael.roth@amd.com wrote:
Add interfaces to allow tests to create/manage SEV guests. The additional state associated with these guests is encapsulated in a new struct sev_vm, which is a light wrapper around struct kvm_vm. These VMs will use vm_set_memory_encryption() and vm_get_encrypted_phy_pages() under the covers to configure and sync up with the core kvm_util library on what should/shouldn't be treated as encrypted memory.
Signed-off-by: Michael Roth michael.roth@amd.com
tools/testing/selftests/kvm/Makefile | 1 + .../selftests/kvm/include/x86_64/sev.h | 62 ++++ tools/testing/selftests/kvm/lib/x86_64/sev.c | 303 ++++++++++++++++++ 3 files changed, 366 insertions(+) create mode 100644 tools/testing/selftests/kvm/include/x86_64/sev.h create mode 100644 tools/testing/selftests/kvm/lib/x86_64/sev.c
diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile index 5832f510a16c..c7a5e1c69e0c 100644 --- a/tools/testing/selftests/kvm/Makefile +++ b/tools/testing/selftests/kvm/Makefile @@ -35,6 +35,7 @@ endif
LIBKVM = lib/assert.c lib/elf.c lib/io.c lib/kvm_util.c lib/rbtree.c lib/sparsebit.c lib/test_util.c lib/guest_modes.c lib/perf_test_util.c LIBKVM_x86_64 = lib/x86_64/apic.c lib/x86_64/processor.c lib/x86_64/vmx.c lib/x86_64/svm.c lib/x86_64/ucall.c lib/x86_64/handlers.S +LIBKVM_x86_64 += lib/x86_64/sev.c
Regarding RFC-level feedback: First off, I'm super jazzed with what I'm seeing so far! (While this is my first review, I've been studying the patches up through the SEV boot test, i.e., patch #7). One thing I'm wondering is: the way this is structured is to essentially split the test cases into non-SEV and SEV. I'm wondering how hard it would be to add some flag or environment variable to set up pre-existing tests to run under SEV. Or is this something you all thought about, and decided that it does not make sense?
Looking at how the guest memory is handled, it seems like it's not far off from handling SEV transparently across all test cases. I'd think that we could just default all memory to use the encryption bit, and then have test cases, such as the test case in patch #7, clear the encryption bit for shared pages. However, I think the VM creation would need a bit more refactoring to work with other test cases.
I think it's possible, but there's a few missing pieces:
As you indicated, existing tests which rely on vm_create(), vm_create_default(), vm_create_default_with_vcpus(), etc. would either need to be updated with whatever new interface provides this 'use-sev' flag, or it would need to happen underneath the covers based on said environment variable/global/etc. There's also the question of where to hook in the sev_vm_launch_start() hooks. Maybe the first time a vcpu_run() is issued? Or maybe some explict call each test will need to be updated to call just prior to initial execution.
Many of the existing tests use the GUESY_SYNC/ucall stuff to handle synchronization between host userspace and guest kernel, which relies on guests issuing PIO instructions to particular port addresses to cause an exit back to host userspace, with various parameters passed via register arguments.
For SEV this would almost work as-is, but some tests might rely on things like memory addresses being passed in this way so would need to audit the code and mark that memory as shared where needed.
For SEV-ES/SEV-SNP, there's a bit more work since:
The registers will not be accessible through the existing KVM_GET_REGS mechanism. It may be possible to set some flag/hook to set/access arguments through some other mechanism like a shared buffer for certain VM types though.
Additionally, the #VC handler only supports CPUID currently, and leverages that fact to avoid doing any significant instruction decoding. Instead the SEV tests use HLT instructions to handle exits to host userspace, which may not work for some tests. So unless there's some other mechanism that SEV/non-SEV tests could utilize rather that PIO, the #VC handler would need to support PIO, which would be nice to have either way, but would likely involve pulling in the intruction decoder library used in the kernel, or some subset/re-implementation of it at least.
Similar to SEV-ES/SEV-SNP requirements for 1), tests which generate PIO/MMIO and other NAE events would need appropriate support for those events in the #VC handler. Nice-to-have either way, but not sure atm how much it would be to implement all of that. Also any tests relying on things like KVM_GET_REGS/KVM_GET_SREGS are non-starters.
One more I should mention:
4) After encryption, the page table is no longer usable for translations by stuff like addr_gva2gpa(), so tests would either need to be audited/updated to do these translations upfront and only rely on cached/stored values thereafter, or perhaps a "shadow" copy could be maintained by kvm_util so the translations will continue to work after encryption.